Add tests for hard and soft links (#12590)

clippy.toml

@@ -10,12 +10,12 @@ doc-valid-idents = [
   "SCREAMING_SNAKE_CASE",
   "SQLAlchemy",
   "StackOverflow",
+  "PyCharm",
 ]
 
 ignore-interior-mutability = [
   # Interned is read-only. The wrapped `Rc` never gets updated.
   "ruff_formatter::format_element::Interned",
-  
+  # The expression is read-only.
-  # The expression is read-only. 
   "ruff_python_ast::hashable::HashableExpr",
 ]

crates/red_knot/Cargo.toml

@@ -27,12 +27,12 @@ notify = { workspace = true }
 rayon = { workspace = true }
 rustc-hash = { workspace = true }
 salsa = { workspace = true }
-filetime = { workspace = true }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true }
 tracing-tree = { workspace = true }
 
 [dev-dependencies]
+filetime = { workspace = true }
 tempfile = { workspace = true }
 
 

crates/red_knot/src/db.rs

@@ -1,7 +1,4 @@
 use std::panic::{AssertUnwindSafe, RefUnwindSafe};
-use std::sync::Arc;
-
-use salsa::Cancelled;
 
 use red_knot_module_resolver::{vendored_typeshed_stubs, Db as ResolverDb};
 use red_knot_python_semantic::Db as SemanticDb;
@@ -10,6 +7,7 @@ use ruff_db::program::{Program, ProgramSettings};
 use ruff_db::system::System;
 use ruff_db::vendored::VendoredFileSystem;
 use ruff_db::{Db as SourceDb, Upcast};
+use salsa::Cancelled;
 
 use crate::lint::Diagnostics;
 use crate::workspace::{check_file, Workspace, WorkspaceMetadata};
@@ -24,7 +22,7 @@ pub struct RootDatabase {
     workspace: Option<Workspace>,
     storage: salsa::Storage<RootDatabase>,
     files: Files,
-    system: Arc<dyn System + Send + Sync + RefUnwindSafe>,
+    system: Box<dyn System + Send + Sync + RefUnwindSafe>,
 }
 
 impl RootDatabase {
@@ -36,7 +34,7 @@ impl RootDatabase {
             workspace: None,
             storage: salsa::Storage::default(),
             files: Files::default(),
-            system: Arc::new(system),
+            system: Box::new(system),
         };
 
         let workspace = Workspace::from_metadata(&db, workspace);

crates/red_knot/tests/file_watching.rs

@@ -1,9 +1,9 @@
 #![allow(clippy::disallowed_names)]
 
+use std::io::Write;
 use std::time::Duration;
 
 use anyhow::{anyhow, Context};
-use filetime::FileTime;
 use salsa::Setter;
 
 use red_knot::db::RootDatabase;
@@ -21,7 +21,10 @@ struct TestCase {
     db: RootDatabase,
     watcher: Option<WorkspaceWatcher>,
     changes_receiver: crossbeam::channel::Receiver<Vec<watch::ChangeEvent>>,
-    temp_dir: tempfile::TempDir,
+    /// The temporary directory that contains the test files.
+    /// We need to hold on to it in the test case or the temp files get deleted.
+    _temp_dir: tempfile::TempDir,
+    root_dir: SystemPathBuf,
 }
 
 impl TestCase {
@@ -30,7 +33,7 @@ impl TestCase {
     }
 
     fn root_path(&self) -> &SystemPath {
-        SystemPath::from_std_path(self.temp_dir.path()).unwrap()
+        &self.root_dir
     }
 
     fn db(&self) -> &RootDatabase {
@@ -42,19 +45,63 @@ impl TestCase {
     }
 
     fn stop_watch(&mut self) -> Vec<watch::ChangeEvent> {
-        if let Some(watcher) = self.watcher.take() {
+        self.try_stop_watch(Duration::from_secs(10))
-            // Give the watcher some time to catch up.
+            .expect("Expected watch changes but observed none.")
-            std::thread::sleep(Duration::from_millis(10));
+    }
+
+    fn try_stop_watch(&mut self, timeout: Duration) -> Option<Vec<watch::ChangeEvent>> {
+        let watcher = self
+            .watcher
+            .take()
+            .expect("Cannot call `stop_watch` more than once.");
+
+        let mut all_events = self
+            .changes_receiver
+            .recv_timeout(timeout)
+            .unwrap_or_default();
+        watcher.flush();
+        watcher.stop();
+
+        for event in &self.changes_receiver {
+            all_events.extend(event);
+        }
+
+        if all_events.is_empty() {
+            return None;
+        }
+
+        Some(all_events)
+    }
+
+    #[cfg(unix)]
+    fn take_watch_changes(&self) -> Vec<watch::ChangeEvent> {
+        self.try_take_watch_changes(Duration::from_secs(10))
+            .expect("Expected watch changes but observed none.")
+    }
+
+    fn try_take_watch_changes(&self, timeout: Duration) -> Option<Vec<watch::ChangeEvent>> {
+        let Some(watcher) = &self.watcher else {
+            return None;
+        };
+
+        let mut all_events = self
+            .changes_receiver
+            .recv_timeout(timeout)
+            .unwrap_or_default();
+        watcher.flush();
+
+        while let Ok(event) = self
+            .changes_receiver
+            .recv_timeout(Duration::from_millis(10))
+        {
+            all_events.extend(event);
             watcher.flush();
-            watcher.stop();
         }
 
-        let mut all_events = Vec::new();
+        if all_events.is_empty() {
-        for events in &self.changes_receiver {
+            return None;
-            all_events.extend(events);
         }
-
+        Some(all_events)
-        all_events
     }
 
     fn update_search_path_settings(
@@ -88,28 +135,62 @@ impl TestCase {
     }
 }
 
-fn setup<I, P>(workspace_files: I) -> anyhow::Result<TestCase>
+trait SetupFiles {
+    fn setup(self, root_path: &SystemPath, workspace_path: &SystemPath) -> anyhow::Result<()>;
+}
+
+impl<const N: usize, P> SetupFiles for [(P, &'static str); N]
 where
-    I: IntoIterator<Item = (P, &'static str)>,
     P: AsRef<SystemPath>,
 {
-    setup_with_search_paths(workspace_files, |_root, workspace_path| {
+    fn setup(self, _root_path: &SystemPath, workspace_path: &SystemPath) -> anyhow::Result<()> {
-        SearchPathSettings {
+        for (relative_path, content) in self {
-            extra_paths: vec![],
+            let relative_path = relative_path.as_ref();
-            workspace_root: workspace_path.to_path_buf(),
+            let absolute_path = workspace_path.join(relative_path);
-            custom_typeshed: None,
+            if let Some(parent) = absolute_path.parent() {
-            site_packages: None,
+                std::fs::create_dir_all(parent).with_context(|| {
+                    format!("Failed to create parent directory for file '{relative_path}'.",)
+                })?;
+            }
+
+            let mut file = std::fs::File::create(absolute_path.as_std_path())
+                .with_context(|| format!("Failed to open file '{relative_path}'"))?;
+            file.write_all(content.as_bytes())
+                .with_context(|| format!("Failed to write to file '{relative_path}'"))?;
+            file.sync_data()?;
         }
+
+        Ok(())
+    }
+}
+
+impl<F> SetupFiles for F
+where
+    F: FnOnce(&SystemPath, &SystemPath) -> anyhow::Result<()>,
+{
+    fn setup(self, root_path: &SystemPath, workspace_path: &SystemPath) -> anyhow::Result<()> {
+        self(root_path, workspace_path)
+    }
+}
+
+fn setup<F>(setup_files: F) -> anyhow::Result<TestCase>
+where
+    F: SetupFiles,
+{
+    setup_with_search_paths(setup_files, |_root, workspace_path| SearchPathSettings {
+        extra_paths: vec![],
+        workspace_root: workspace_path.to_path_buf(),
+        custom_typeshed: None,
+        site_packages: None,
     })
 }
 
-fn setup_with_search_paths<I, P>(
+fn setup_with_search_paths<F>(
-    workspace_files: I,
+    setup_files: F,
     create_search_paths: impl FnOnce(&SystemPath, &SystemPath) -> SearchPathSettings,
 ) -> anyhow::Result<TestCase>
 where
-    I: IntoIterator<Item = (P, &'static str)>,
+    F: SetupFiles,
-    P: AsRef<SystemPath>,
 {
     let temp_dir = tempfile::tempdir()?;
 
@@ -132,18 +213,9 @@ where
     std::fs::create_dir_all(workspace_path.as_std_path())
         .with_context(|| format!("Failed to create workspace directory '{workspace_path}'",))?;
 
-    for (relative_path, content) in workspace_files {
+    setup_files
-        let relative_path = relative_path.as_ref();
+        .setup(&root_path, &workspace_path)
-        let absolute_path = workspace_path.join(relative_path);
+        .context("Failed to setup test files")?;
-        if let Some(parent) = absolute_path.parent() {
-            std::fs::create_dir_all(parent).with_context(|| {
-                format!("Failed to create parent directory for file '{relative_path}'.",)
-            })?;
-        }
-
-        std::fs::write(absolute_path.as_std_path(), content)
-            .with_context(|| format!("Failed to write file '{relative_path}'"))?;
-    }
 
     let system = OsSystem::new(&workspace_path);
 
@@ -178,30 +250,43 @@ where
         db,
         changes_receiver: receiver,
         watcher: Some(watcher),
-        temp_dir,
+        _temp_dir: temp_dir,
+        root_dir: root_path,
     };
 
+    // Sometimes the file watcher reports changes for events that happened before the watcher was started.
+    // Do a best effort at dropping these events.
+    test_case.try_take_watch_changes(Duration::from_millis(100));
+
     Ok(test_case)
 }
 
-/// The precision of the last modified time is platform dependent and not arbitrarily precise.
+/// Updates the content of a file and ensures that the last modified file time is updated.
-/// This method sleeps until the last modified time of a newly created file changes. This guarantees
+fn update_file(path: impl AsRef<SystemPath>, content: &str) -> anyhow::Result<()> {
-/// that the last modified time of any file written **after** this method completes should be different.
+    let path = path.as_ref().as_std_path();
-fn next_io_tick() {
-    let temp = tempfile::tempfile().unwrap();
 
-    let last_modified = FileTime::from_last_modification_time(&temp.metadata().unwrap());
+    let metadata = path.metadata()?;
+    let last_modified_time = filetime::FileTime::from_last_modification_time(&metadata);
+
+    let mut file = std::fs::OpenOptions::new()
+        .create(false)
+        .write(true)
+        .truncate(true)
+        .open(path)?;
+    file.write_all(content.as_bytes())?;
 
     loop {
-        filetime::set_file_handle_times(&temp, None, Some(FileTime::now())).unwrap();
+        file.sync_all()?;
 
-        let new_last_modified = FileTime::from_last_modification_time(&temp.metadata().unwrap());
+        let modified_time = filetime::FileTime::from_last_modification_time(&path.metadata()?);
 
-        if new_last_modified != last_modified {
+        if modified_time != last_modified_time {
-            break;
+            break Ok(());
         }
 
-        std::thread::sleep(Duration::from_nanos(100));
+        std::thread::sleep(Duration::from_nanos(10));
+
+        filetime::set_file_handle_times(&file, None, Some(filetime::FileTime::now()))?;
     }
 }
 
@@ -260,8 +345,7 @@ fn changed_file() -> anyhow::Result<()> {
     assert_eq!(source_text(case.db(), foo).as_str(), foo_source);
     assert_eq!(&case.collect_package_files(&foo_path), &[foo]);
 
-    next_io_tick();
+    update_file(&foo_path, "print('Version 2')")?;
-    std::fs::write(foo_path.as_std_path(), "print('Version 2')")?;
 
     let changes = case.stop_watch();
 
@@ -275,49 +359,6 @@ fn changed_file() -> anyhow::Result<()> {
     Ok(())
 }
 
-#[cfg(unix)]
-#[test]
-fn changed_metadata() -> anyhow::Result<()> {
-    use std::os::unix::fs::PermissionsExt;
-
-    let mut case = setup([("foo.py", "")])?;
-    let foo_path = case.workspace_path("foo.py");
-
-    let foo = case.system_file(&foo_path)?;
-    assert_eq!(
-        foo.permissions(case.db()),
-        Some(
-            std::fs::metadata(foo_path.as_std_path())
-                .unwrap()
-                .permissions()
-                .mode()
-        )
-    );
-
-    next_io_tick();
-    std::fs::set_permissions(
-        foo_path.as_std_path(),
-        std::fs::Permissions::from_mode(0o777),
-    )
-    .with_context(|| "Failed to set file permissions.")?;
-
-    let changes = case.stop_watch();
-
-    case.db_mut().apply_changes(changes);
-
-    assert_eq!(
-        foo.permissions(case.db()),
-        Some(
-            std::fs::metadata(foo_path.as_std_path())
-                .unwrap()
-                .permissions()
-                .mode()
-        )
-    );
-
-    Ok(())
-}
-
 #[test]
 fn deleted_file() -> anyhow::Result<()> {
     let foo_source = "print('Hello, world!')";
@@ -495,7 +536,7 @@ fn directory_moved_to_trash() -> anyhow::Result<()> {
     ])?;
     let bar = case.system_file(case.workspace_path("bar.py")).unwrap();
 
-    assert!(resolve_module(case.db().upcast(), ModuleName::new_static("sub.a").unwrap()).is_some(),);
+    assert!(resolve_module(case.db().upcast(), ModuleName::new_static("sub.a").unwrap()).is_some());
 
     let sub_path = case.workspace_path("sub");
     let init_file = case
@@ -729,9 +770,464 @@ fn remove_search_path() -> anyhow::Result<()> {
 
     std::fs::write(site_packages.join("a.py").as_std_path(), "class A: ...")?;
 
-    let changes = case.stop_watch();
+    let changes = case.try_stop_watch(Duration::from_millis(100));
 
-    assert_eq!(changes, &[]);
+    assert_eq!(changes, None);
 
     Ok(())
 }
+
+/// Watch a workspace that contains two files where one file is a hardlink to another.
+///
+/// Setup:
+/// ```text
+/// - workspace
+///   |- foo.py
+///   |- bar.py (hard link to foo.py)
+/// ```
+///
+/// # Linux
+/// `inotify` only emits a single change event for the file that was changed.
+/// Other files that point to the same inode (hardlinks) won't get updated.
+///
+/// For reference: VS Code and PyCharm have the same behavior where the results for one of the
+/// files are stale.
+///
+/// # Windows
+/// I haven't found any documentation that states the notification behavior on Windows but what
+/// we're seeing is that Windows only emits a single event, similar to Linux.
+#[test]
+fn hard_links_in_workspace() -> anyhow::Result<()> {
+    let mut case = setup(|_root: &SystemPath, workspace: &SystemPath| {
+        let foo_path = workspace.join("foo.py");
+        std::fs::write(foo_path.as_std_path(), "print('Version 1')")?;
+
+        // Create a hardlink to `foo`
+        let bar_path = workspace.join("bar.py");
+        std::fs::hard_link(foo_path.as_std_path(), bar_path.as_std_path())
+            .context("Failed to create hard link from foo.py -> bar.py")?;
+
+        Ok(())
+    })?;
+
+    let foo_path = case.workspace_path("foo.py");
+    let foo = case.system_file(&foo_path).unwrap();
+    let bar_path = case.workspace_path("bar.py");
+    let bar = case.system_file(&bar_path).unwrap();
+
+    assert_eq!(source_text(case.db(), foo).as_str(), "print('Version 1')");
+    assert_eq!(source_text(case.db(), bar).as_str(), "print('Version 1')");
+
+    // Write to the hard link target.
+    update_file(foo_path, "print('Version 2')").context("Failed to update foo.py")?;
+
+    let changes = case.stop_watch();
+
+    case.db_mut().apply_changes(changes);
+
+    assert_eq!(source_text(case.db(), foo).as_str(), "print('Version 2')");
+
+    // macOS is the only platform that emits events for every hardlink.
+    if cfg!(target_os = "macos") {
+        assert_eq!(source_text(case.db(), bar).as_str(), "print('Version 2')");
+    }
+
+    Ok(())
+}
+
+/// Watch a workspace that contains one file that is a hardlink to a file outside the workspace.
+///
+/// Setup:
+/// ```text
+/// - foo.py
+/// - workspace
+///   |- bar.py (hard link to /foo.py)
+/// ```
+///
+/// # Linux
+/// inotiyf doesn't support observing changes to hard linked files.
+///
+/// > Note: when monitoring a directory, events are not generated for
+/// > the files inside the directory when the events are performed via
+/// > a pathname (i.e., a link) that lies outside the monitored
+/// > directory. [source](https://man7.org/linux/man-pages/man7/inotify.7.html)
+///
+/// # Windows
+/// > Retrieves information that describes the changes within the specified directory.
+///
+/// [source](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-readdirectorychangesw)
+///
+/// My interpretation of this is that Windows doesn't support observing changes made to
+/// hard linked files outside the workspace.
+#[test]
+#[cfg_attr(
+    target_os = "linux",
+    ignore = "inotify doesn't support observing changes to hard linked files."
+)]
+#[cfg_attr(
+    target_os = "windows",
+    ignore = "windows doesn't support observing changes to hard linked files."
+)]
+fn hard_links_to_target_outside_workspace() -> anyhow::Result<()> {
+    let mut case = setup(|root: &SystemPath, workspace: &SystemPath| {
+        let foo_path = root.join("foo.py");
+        std::fs::write(foo_path.as_std_path(), "print('Version 1')")?;
+
+        // Create a hardlink to `foo`
+        let bar_path = workspace.join("bar.py");
+        std::fs::hard_link(foo_path.as_std_path(), bar_path.as_std_path())
+            .context("Failed to create hard link from foo.py -> bar.py")?;
+
+        Ok(())
+    })?;
+
+    let foo_path = case.root_path().join("foo.py");
+    let foo = case.system_file(&foo_path).unwrap();
+    let bar_path = case.workspace_path("bar.py");
+    let bar = case.system_file(&bar_path).unwrap();
+
+    assert_eq!(source_text(case.db(), foo).as_str(), "print('Version 1')");
+    assert_eq!(source_text(case.db(), bar).as_str(), "print('Version 1')");
+
+    // Write to the hard link target.
+    update_file(foo_path, "print('Version 2')").context("Failed to update foo.py")?;
+
+    let changes = case.stop_watch();
+
+    case.db_mut().apply_changes(changes);
+
+    assert_eq!(source_text(case.db(), bar).as_str(), "print('Version 2')");
+
+    Ok(())
+}
+
+#[cfg(unix)]
+mod unix {
+    //! Tests that make use of unix specific file-system features.
+    use super::*;
+
+    /// Changes the metadata of the only file in the workspace.
+    #[test]
+    fn changed_metadata() -> anyhow::Result<()> {
+        use std::os::unix::fs::PermissionsExt;
+
+        let mut case = setup([("foo.py", "")])?;
+        let foo_path = case.workspace_path("foo.py");
+
+        let foo = case.system_file(&foo_path)?;
+        assert_eq!(
+            foo.permissions(case.db()),
+            Some(
+                std::fs::metadata(foo_path.as_std_path())
+                    .unwrap()
+                    .permissions()
+                    .mode()
+            )
+        );
+
+        std::fs::set_permissions(
+            foo_path.as_std_path(),
+            std::fs::Permissions::from_mode(0o777),
+        )
+        .with_context(|| "Failed to set file permissions.")?;
+
+        let changes = case.stop_watch();
+
+        case.db_mut().apply_changes(changes);
+
+        assert_eq!(
+            foo.permissions(case.db()),
+            Some(
+                std::fs::metadata(foo_path.as_std_path())
+                    .unwrap()
+                    .permissions()
+                    .mode()
+            )
+        );
+
+        Ok(())
+    }
+
+    /// A workspace path is a symlink to a file outside the workspace.
+    ///
+    /// Setup:
+    /// ```text
+    /// - bar
+    ///   |- baz.py
+    ///
+    /// - workspace
+    ///   |- bar -> /bar
+    /// ```
+    ///
+    /// # macOS
+    /// This test case isn't supported on macOS.
+    /// macOS uses `FSEvents` and `FSEvents` doesn't emit an event if a file in a symlinked directory is changed.
+    ///
+    /// > Generally speaking, when working with file system event notifications, you will probably want to use lstat,
+    /// > because changes to the underlying file will not result in a change notification for the directory containing
+    /// > the symbolic link to that file. However, if you are working with a controlled file structure in
+    /// > which symbolic links always point within your watched tree, you might have reason to use stat.
+    ///
+    /// [source](https://developer.apple.com/library/archive/documentation/Darwin/Conceptual/FSEvents_ProgGuide/UsingtheFSEventsFramework/UsingtheFSEventsFramework.html#//apple_ref/doc/uid/TP40005289-CH4-SW4)
+    ///
+    /// Pyright also does not support this case.
+    #[test]
+    #[cfg_attr(
+        target_os = "macos",
+        ignore = "FSEvents doesn't emit change events for symlinked directories outside of the watched paths."
+    )]
+    fn symlink_target_outside_watched_paths() -> anyhow::Result<()> {
+        let mut case = setup(|root: &SystemPath, workspace: &SystemPath| {
+            // Set up the symlink target.
+            let link_target = root.join("bar");
+            std::fs::create_dir_all(link_target.as_std_path())
+                .context("Failed to create link target directory")?;
+            let baz_original = link_target.join("baz.py");
+            std::fs::write(baz_original.as_std_path(), "def baz(): ...")
+                .context("Failed to write link target file")?;
+
+            // Create a symlink inside the workspace
+            let bar = workspace.join("bar");
+            std::os::unix::fs::symlink(link_target.as_std_path(), bar.as_std_path())
+                .context("Failed to create symlink to bar package")?;
+
+            Ok(())
+        })?;
+
+        let baz = resolve_module(
+            case.db().upcast(),
+            ModuleName::new_static("bar.baz").unwrap(),
+        )
+        .expect("Expected bar.baz to exist in site-packages.");
+        let baz_workspace = case.workspace_path("bar/baz.py");
+
+        assert_eq!(
+            source_text(case.db(), baz.file()).as_str(),
+            "def baz(): ..."
+        );
+        assert_eq!(
+            baz.file().path(case.db()).as_system_path(),
+            Some(&*baz_workspace)
+        );
+
+        let baz_original = case.root_path().join("bar/baz.py");
+
+        // Write to the symlink target.
+        update_file(baz_original, "def baz(): print('Version 2')")
+            .context("Failed to update bar/baz.py")?;
+
+        let changes = case.take_watch_changes();
+
+        case.db_mut().apply_changes(changes);
+
+        assert_eq!(
+            source_text(case.db(), baz.file()).as_str(),
+            "def baz(): print('Version 2')"
+        );
+
+        // Write to the symlink source.
+        update_file(baz_workspace, "def baz(): print('Version 3')")
+            .context("Failed to update bar/baz.py")?;
+
+        let changes = case.stop_watch();
+
+        case.db_mut().apply_changes(changes);
+
+        assert_eq!(
+            source_text(case.db(), baz.file()).as_str(),
+            "def baz(): print('Version 3')"
+        );
+
+        Ok(())
+    }
+
+    /// Workspace contains a symlink to another directory inside the workspace.
+    /// Changes to files in the symlinked directory should be reflected
+    /// to all files.
+    ///
+    /// Setup:
+    /// ```text
+    /// - workspace
+    ///   | - bar -> /workspace/patched/bar
+    ///   |
+    ///   | - patched
+    ///   |   |-- bar
+    ///   |   |  |- baz.py
+    ///   |
+    ///   |-- foo.py
+    /// ```
+    #[test]
+    fn symlink_inside_workspace() -> anyhow::Result<()> {
+        let mut case = setup(|_root: &SystemPath, workspace: &SystemPath| {
+            // Set up the symlink target.
+            let link_target = workspace.join("patched/bar");
+            std::fs::create_dir_all(link_target.as_std_path())
+                .context("Failed to create link target directory")?;
+            let baz_original = link_target.join("baz.py");
+            std::fs::write(baz_original.as_std_path(), "def baz(): ...")
+                .context("Failed to write link target file")?;
+
+            // Create a symlink inside site-packages
+            let bar_in_workspace = workspace.join("bar");
+            std::os::unix::fs::symlink(link_target.as_std_path(), bar_in_workspace.as_std_path())
+                .context("Failed to create symlink to bar package")?;
+
+            Ok(())
+        })?;
+
+        let baz = resolve_module(
+            case.db().upcast(),
+            ModuleName::new_static("bar.baz").unwrap(),
+        )
+        .expect("Expected bar.baz to exist in site-packages.");
+        let bar_baz = case.workspace_path("bar/baz.py");
+
+        let patched_bar_baz = case.workspace_path("patched/bar/baz.py");
+        let patched_bar_baz_file = case.system_file(&patched_bar_baz).unwrap();
+
+        assert_eq!(
+            source_text(case.db(), patched_bar_baz_file).as_str(),
+            "def baz(): ..."
+        );
+
+        assert_eq!(
+            source_text(case.db(), baz.file()).as_str(),
+            "def baz(): ..."
+        );
+        assert_eq!(baz.file().path(case.db()).as_system_path(), Some(&*bar_baz));
+
+        // Write to the symlink target.
+        update_file(&patched_bar_baz, "def baz(): print('Version 2')")
+            .context("Failed to update bar/baz.py")?;
+
+        let changes = case.stop_watch();
+
+        case.db_mut().apply_changes(changes);
+
+        // The file watcher is guaranteed to emit one event for the changed file, but it isn't specified
+        // if the event is emitted for the "original" or linked path because both paths are watched.
+        // The best we can assert here is that one of the files should have been updated.
+        //
+        // In a perfect world, the file watcher would emit two events, one for the original file and
+        // one for the symlink. I tried parcel/watcher, node's `fs.watch` and `chokidar` and
+        // only `chokidar seems to support it (used by Pyright).
+        //
+        // I further tested how good editor support is for symlinked files and it is not good ;)
+        // * VS Code doesn't update the file content if a file gets changed through a symlink
+        // * PyCharm doesn't update diagnostics if a symlinked module is changed (same as red knot).
+        //
+        // That's why I think it's fine to not support this case for now.
+
+        let patched_baz_text = source_text(case.db(), patched_bar_baz_file);
+        let did_update_patched_baz = patched_baz_text.as_str() == "def baz(): print('Version 2')";
+
+        let bar_baz_text = source_text(case.db(), baz.file());
+        let did_update_bar_baz = bar_baz_text.as_str() == "def baz(): print('Version 2')";
+
+        assert!(
+            did_update_patched_baz || did_update_bar_baz,
+            "Expected one of the files to be updated but neither file was updated.\nOriginal: {patched_baz_text}\nSymlinked: {bar_baz_text}",
+            patched_baz_text = patched_baz_text.as_str(),
+            bar_baz_text = bar_baz_text.as_str()
+        );
+
+        Ok(())
+    }
+
+    /// A module search path is a symlink.
+    ///
+    /// Setup:
+    /// ```text
+    /// - site-packages
+    ///   | - bar/baz.py
+    ///
+    /// - workspace
+    ///   |-- .venv/lib/python3.12/site-packages -> /site-packages
+    ///   |
+    ///   |-- foo.py
+    /// ```
+    #[test]
+    fn symlinked_module_search_path() -> anyhow::Result<()> {
+        let mut case = setup_with_search_paths(
+            |root: &SystemPath, workspace: &SystemPath| {
+                // Set up the symlink target.
+                let site_packages = root.join("site-packages");
+                let bar = site_packages.join("bar");
+                std::fs::create_dir_all(bar.as_std_path())
+                    .context("Failed to create bar directory")?;
+                let baz_original = bar.join("baz.py");
+                std::fs::write(baz_original.as_std_path(), "def baz(): ...")
+                    .context("Failed to write baz.py")?;
+
+                // Symlink the site packages in the venv to the global site packages
+                let venv_site_packages = workspace.join(".venv/lib/python3.12/site-packages");
+                std::fs::create_dir_all(venv_site_packages.parent().unwrap())
+                    .context("Failed to create .venv directory")?;
+                std::os::unix::fs::symlink(
+                    site_packages.as_std_path(),
+                    venv_site_packages.as_std_path(),
+                )
+                .context("Failed to create symlink to site-packages")?;
+
+                Ok(())
+            },
+            |_root, workspace| SearchPathSettings {
+                extra_paths: vec![],
+                workspace_root: workspace.to_path_buf(),
+                custom_typeshed: None,
+                site_packages: Some(workspace.join(".venv/lib/python3.12/site-packages")),
+            },
+        )?;
+
+        let baz = resolve_module(
+            case.db().upcast(),
+            ModuleName::new_static("bar.baz").unwrap(),
+        )
+        .expect("Expected bar.baz to exist in site-packages.");
+        let baz_site_packages =
+            case.workspace_path(".venv/lib/python3.12/site-packages/bar/baz.py");
+        let baz_original = case.root_path().join("site-packages/bar/baz.py");
+        let baz_original_file = case.system_file(&baz_original).unwrap();
+
+        assert_eq!(
+            source_text(case.db(), baz_original_file).as_str(),
+            "def baz(): ..."
+        );
+
+        assert_eq!(
+            source_text(case.db(), baz.file()).as_str(),
+            "def baz(): ..."
+        );
+        assert_eq!(
+            baz.file().path(case.db()).as_system_path(),
+            Some(&*baz_site_packages)
+        );
+
+        // Write to the symlink target.
+        update_file(&baz_original, "def baz(): print('Version 2')")
+            .context("Failed to update bar/baz.py")?;
+
+        let changes = case.stop_watch();
+
+        case.db_mut().apply_changes(changes);
+
+        assert_eq!(
+            source_text(case.db(), baz.file()).as_str(),
+            "def baz(): print('Version 2')"
+        );
+
+        // It would be nice if this is supported but the underlying file system watchers
+        // only emit a single event. For reference
+        // * VS Code doesn't update the file content if a file gets changed through a symlink
+        // * PyCharm doesn't update diagnostics if a symlinked module is changed (same as red knot).
+        // We could add support for it by keeping a reverse map from `real_path` to symlinked path but
+        // it doesn't seem worth doing considering that as prominent tools like PyCharm don't support it.
+        // Pyright does support it, thanks to chokidar.
+        assert_ne!(
+            source_text(case.db(), baz_original_file).as_str(),
+            "def baz(): print('Version 2')"
+        );
+
+        Ok(())
+    }
+}

crates/red_knot_module_resolver/src/resolver.rs

@@ -2,12 +2,11 @@ use std::borrow::Cow;
 use std::iter::FusedIterator;
 
 use once_cell::sync::Lazy;
-use rustc_hash::{FxBuildHasher, FxHashSet};
-
 use ruff_db::files::{File, FilePath, FileRootKind};
 use ruff_db::program::{Program, SearchPathSettings, TargetVersion};
 use ruff_db::system::{DirectoryEntry, System, SystemPath, SystemPathBuf};
 use ruff_db::vendored::VendoredPath;
+use rustc_hash::{FxBuildHasher, FxHashSet};
 
 use crate::db::Db;
 use crate::module::{Module, ModuleKind};
@@ -625,13 +624,12 @@ impl PackageKind {
 #[cfg(test)]
 mod tests {
     use ruff_db::files::{system_path_to_file, File, FilePath};
-    use ruff_db::system::{DbWithTestSystem, OsSystem, SystemPath};
+    use ruff_db::system::DbWithTestSystem;
     use ruff_db::testing::{
         assert_const_function_query_was_not_run, assert_function_query_was_not_run,
     };
     use ruff_db::Db;
 
-    use crate::db::tests::TestDb;
     use crate::module::ModuleKind;
     use crate::module_name::ModuleName;
     use crate::testing::{FileSpec, MockedTypeshed, TestCase, TestCaseBuilder};
@@ -1153,7 +1151,9 @@ mod tests {
     #[test]
     #[cfg(target_family = "unix")]
     fn symlink() -> anyhow::Result<()> {
+        use crate::db::tests::TestDb;
         use ruff_db::program::Program;
+        use ruff_db::system::{OsSystem, SystemPath};
 
         let mut db = TestDb::new();
 

crates/ruff_db/src/system.rs

@@ -39,6 +39,8 @@ pub type Result<T> = std::io::Result<T>;
 /// Abstracting the system also enables tests to use a more efficient in-memory file system.
 pub trait System: Debug {
     /// Reads the metadata of the file or directory at `path`.
+    ///
+    /// This function will traverse symbolic links to query information about the destination file.
     fn path_metadata(&self, path: &SystemPath) -> Result<Metadata>;
 
     /// Reads the content of the file at `path` into a [`String`].