More integration tests

Author: MichaReiser

Cargo.lock

@@ -13,6 +13,7 @@ poit = "poit"
 BA = "BA" # acronym for "Bad Allowed", used in testing.
 jod = "jod" # e.g., `jod-thread`
 Numer = "Numer" # Library name 'NumerBlox' in "Who's Using Ruff?"
+"FrIeAndLy-._.-bArD" = "FrIeAndLy-._.-bArD"
 
 [default]
 extend-ignore-re = [

_typos.toml

@@ -6,7 +6,7 @@ use std::time::Duration;
 use anyhow::{anyhow, Context};
 
 use red_knot_python_semantic::{resolve_module, ModuleName, Program, PythonVersion, SitePackages};
-use red_knot_workspace::db::RootDatabase;
+use red_knot_workspace::db::{Db, RootDatabase};
 use red_knot_workspace::watch;
 use red_knot_workspace::watch::{directory_watcher, WorkspaceWatcher};
 use red_knot_workspace::workspace::settings::{Configuration, SearchPathConfiguration};
@@ -1310,3 +1310,42 @@ mod unix {
         Ok(())
     }
 }
+
+#[test]
+fn nested_packages_delete_root() -> anyhow::Result<()> {
+    let mut case = setup(|root: &SystemPath, workspace_root: &SystemPath| {
+        std::fs::write(
+            workspace_root.join("pyproject.toml").as_std_path(),
+            r#"
+            [project]
+            name = "inner"
+            "#,
+        )?;
+
+        std::fs::write(
+            root.join("pyproject.toml").as_std_path(),
+            r#"
+            [project]
+            name = "outer"
+            "#,
+        )?;
+
+        Ok(())
+    })?;
+
+    assert_eq!(
+        case.db().workspace().root(case.db()),
+        &*case.workspace_path("")
+    );
+
+    std::fs::remove_file(case.workspace_path("pyproject.toml").as_std_path())?;
+
+    let changes = case.stop_watch();
+
+    case.apply_changes(changes);
+
+    // It should now pick up the outer workspace.
+    assert_eq!(case.db().workspace().root(case.db()), case.root_path());
+
+    Ok(())
+}

crates/red_knot/tests/file_watching.rs

@@ -3,7 +3,7 @@ use std::any::Any;
 use js_sys::Error;
 use wasm_bindgen::prelude::*;
 
-use red_knot_workspace::db::RootDatabase;
+use red_knot_workspace::db::{Db, RootDatabase};
 use red_knot_workspace::workspace::settings::Configuration;
 use red_knot_workspace::workspace::WorkspaceMetadata;
 use ruff_db::diagnostic::Diagnostic;

crates/red_knot_wasm/src/lib.rs

@@ -35,6 +35,7 @@ tracing = { workspace = true }
 
 [dev-dependencies]
 ruff_db = { workspace = true, features = ["testing"] }
+insta = { workspace = true }
 tempfile = { workspace = true }
 
 [features]

crates/red_knot_workspace/Cargo.toml

@@ -15,7 +15,9 @@ use ruff_db::{Db as SourceDb, Upcast};
 mod changes;
 
 #[salsa::db]
-pub trait Db: SemanticDb + Upcast<dyn SemanticDb> {}
+pub trait Db: SemanticDb + Upcast<dyn SemanticDb> {
+    fn workspace(&self) -> Workspace;
+}
 
 #[salsa::db]
 pub struct RootDatabase {
@@ -38,39 +40,13 @@ impl RootDatabase {
         };
 
         // Initialize the `Program` singleton
-
-        // TODO: Reasoning: We need to know the target version at this point to
-        // load the correct persistent cache so that running knot with
-        // different target versions doesn't result in 0 cache-reuse.
-        // But this does complicate things a bit because we need to have the
-        // resolved workspace settings at this point.
-        // But resolving the settings fits well into the workspace's responsibility.
-        // Should `Workspace` initialize the Program -> No, because we then need to create
-        // the program first.
-
-        // Metadata: Plain description of the workspace and the exact configuration (without doing any resolution)
-        // Workspace: Resolved workspace with all members, merged settings.
-        // Middle ground -> `metadata.resolve_settings(configuration)` returns the resolved
-        // `.workspace` configuration but it doesn't resolve any of the members configuration.
-        // The problem with this is that it makes the single project use case awkward. We should just
-        // resolve the program settings and defer everything else to later? But how does this work with
-        // with target versions that are inherited from the workspace?
-        // So this is somewhat awkward too :(
-        // The other option is that `WorkspaceMetadata` keeps working as it is today where it stores the
-        // resolved settings instead of the unresolved. I do thik that this would require another intermediate representation
-        // to not be awkward.
         Program::from_settings(&db, workspace.settings().program())?;
 
         db.workspace = Some(Workspace::from_metadata(&db, workspace));
 
         Ok(db)
     }
 
-    pub fn workspace(&self) -> Workspace {
-        // SAFETY: The workspace is always initialized in `new`.
-        self.workspace.unwrap()
-    }
-
     /// Checks all open files in the workspace and its dependencies.
     pub fn check(&self) -> Result<Vec<Box<dyn Diagnostic>>, Cancelled> {
         self.with_db(|db| db.workspace().check(db))
@@ -174,7 +150,11 @@ impl salsa::Database for RootDatabase {
 }
 
 #[salsa::db]
-impl Db for RootDatabase {}
+impl Db for RootDatabase {
+    fn workspace(&self) -> Workspace {
+        self.workspace.unwrap()
+    }
+}
 
 #[cfg(test)]
 pub(crate) mod tests {
@@ -189,6 +169,7 @@ pub(crate) mod tests {
     use ruff_db::{Db as SourceDb, Upcast};
 
     use crate::db::Db;
+    use crate::workspace::Workspace;
 
     #[salsa::db]
     pub(crate) struct TestDb {
@@ -275,7 +256,11 @@ pub(crate) mod tests {
     }
 
     #[salsa::db]
-    impl Db for TestDb {}
+    impl Db for TestDb {
+        fn workspace(&self) -> Workspace {
+            todo!()
+        }
+    }
 
     #[salsa::db]
     impl salsa::Database for TestDb {

crates/red_knot_workspace/src/db.rs

@@ -1,24 +1,23 @@
+use crate::db::{Db, RootDatabase};
+use crate::watch;
+use crate::watch::{CreatedKind, DeletedKind};
+use crate::workspace::settings::Configuration;
+use crate::workspace::{Workspace, WorkspaceMetadata};
 use red_knot_python_semantic::Program;
 use ruff_db::files::{system_path_to_file, File, Files};
 use ruff_db::system::walk_directory::WalkState;
 use ruff_db::system::SystemPath;
-use ruff_db::Db;
+use ruff_db::Db as _;
 use rustc_hash::FxHashSet;
 
-use crate::db::RootDatabase;
-use crate::watch;
-use crate::watch::{CreatedKind, DeletedKind};
-use crate::workspace::settings::Configuration;
-use crate::workspace::WorkspaceMetadata;
-
 impl RootDatabase {
     #[tracing::instrument(level = "debug", skip(self, changes, base_configuration))]
     pub fn apply_changes(
         &mut self,
         changes: Vec<watch::ChangeEvent>,
         base_configuration: Option<&Configuration>,
     ) {
-        let workspace = self.workspace();
+        let mut workspace = self.workspace();
         let workspace_path = workspace.root(self).to_path_buf();
         let program = Program::get(self);
         let custom_stdlib_versions_path = program
@@ -58,6 +57,10 @@ impl RootDatabase {
                     // Changes to ignore files or settings can change the workspace structure or add/remove files
                     // from packages.
                     if let Some(package) = workspace.package(self, path) {
+                        if package.root(self) == workspace.root(self) {
+                            workspace_change = true;
+                        }
+
                         changed_packages.insert(package);
                     } else {
                         workspace_change = true;
@@ -153,12 +156,20 @@ impl RootDatabase {
         if workspace_change {
             match WorkspaceMetadata::discover(&workspace_path, self.system(), base_configuration) {
                 Ok(metadata) => {
-                    tracing::debug!("Reloading workspace after structural change");
-                    // TODO: Handle changes in the program settings.
-                    workspace.reload(self, metadata);
+                    if metadata.root() == workspace.root(self) {
+                        tracing::debug!("Reloading workspace after structural change");
+                        // TODO: Handle changes in the program settings.
+                        workspace.reload(self, metadata);
+                    } else {
+                        tracing::debug!("Replace workspace after structural change");
+                        workspace = Workspace::from_metadata(self, metadata);
+                        self.workspace = Some(workspace);
+                    }
                 }
                 Err(error) => {
-                    tracing::error!("Failed to load workspace, keep old workspace: {error}");
+                    tracing::error!(
+                        "Failed to load workspace, keeping old workspace configuration: {error}"
+                    );
                 }
             }
 
@@ -223,6 +234,3 @@ impl RootDatabase {
         }
     }
 }
-
-#[cfg(test)]
-mod tests {}

crates/red_knot_workspace/src/db/changes.rs

@@ -8,7 +8,7 @@ use ruff_cache::{CacheKey, CacheKeyHasher};
 use ruff_db::system::{SystemPath, SystemPathBuf};
 use ruff_db::Upcast;
 
-use crate::db::RootDatabase;
+use crate::db::{Db, RootDatabase};
 use crate::watch::Watcher;
 
 /// Wrapper around a [`Watcher`] that watches the relevant paths of a workspace.