[red-knot] Case sensitive module resolver (#16521)

## Summary

This PR implements the first part of
#16440. It ensures that Red
Knot's module resolver is case sensitive on all systems.

This PR combines a few approaches:

1. It uses `canonicalize` on non-case-sensitive systems to get the real
casing of a path. This works for as long as no symlinks or mapped
network drives (the windows `E:\` is mapped to `\\server\share` thingy).
This is the same as what Pyright does
2. If 1. fails, fall back to recursively list the parent directory and
test if the path's file name matches the casing exactly as listed in by
list dir. This is the same approach as CPython takes in its module
resolver. The main downside is that it requires more syscalls because,
unlike CPython, we Red Knot needs to invalidate its caches if a file
name gets renamed (CPython assumes that the folders are immutable).

It's worth noting that the file watching test that I added that renames
`lib.py` to `Lib.py` currently doesn't pass on case-insensitive systems.
Making it pass requires some more involved changes to `Files`. I plan to
work on this next. There's the argument that landing this PR on its own
isn't worth it without this issue being addressed. I think it's still a
good step in the right direction even when some of the details on how
and where the path case sensitive comparison is implemented.

## Test plan

I added multiple integration tests (including a failing one). I tested
that the `case-sensitivity` detection works as expected on Windows,
MacOS and Linux and that the fast-paths are taken accordingly.

Author: MichaReiser

crates/red_knot/src/main.rs

@@ -80,6 +80,8 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> {
     countme::enable(verbosity.is_trace());
     let _guard = setup_tracing(verbosity)?;
 
+    tracing::debug!("Version: {}", version::version());
+
     // The base path to which all CLI arguments are relative to.
     let cwd = {
         let cwd = std::env::current_dir().context("Failed to get the current working directory")?;

crates/red_knot/tests/file_watching.rs

@@ -1,5 +1,3 @@
-#![allow(clippy::disallowed_names)]
-
 use std::collections::HashSet;
 use std::io::Write;
 use std::time::{Duration, Instant};
@@ -16,7 +14,7 @@ use ruff_db::source::source_text;
 use ruff_db::system::{
     OsSystem, System, SystemPath, SystemPathBuf, UserConfigDirectoryOverrideGuard,
 };
-use ruff_db::Upcast;
+use ruff_db::{Db as _, Upcast};
 use ruff_python_ast::PythonVersion;
 
 struct TestCase {
@@ -1790,3 +1788,82 @@ fn changes_to_user_configuration() -> anyhow::Result<()> {
 
     Ok(())
 }
+
+/// Tests that renaming a file from `lib.py` to `Lib.py` is correctly reflected.
+///
+/// This test currently fails on case-insensitive systems because `Files` is case-sensitive
+/// but the `System::metadata` call isn't. This means that
+/// Red Knot considers both `Lib.py` and `lib.py` to exist when only `lib.py` does
+///
+/// The incoming change events then are no-ops because they don't change either file's
+/// status nor does it update their last modified time (renaming a file doesn't bump it's
+/// last modified timestamp).
+///
+/// Fixing this requires to either make `Files` case-insensitive and store the
+/// real-case path (if it differs) on `File` or make `Files` use a
+/// case-sensitive `System::metadata` call. This does open the question if all
+/// `System` calls should be case sensitive. This would be the most consistent
+/// but might be hard to pull off.
+///
+/// What the right solution is also depends on if Red Knot itself should be case
+/// sensitive or not. E.g. should `include="src"` be case sensitive on all systems
+/// or only on case-sensitive systems?
+///
+/// Lastly, whatever solution we pick must also work well with VS Code which,
+/// unfortunately ,doesn't propagate casing-only renames.
+/// <https://github.com/rust-lang/rust-analyzer/issues/9581>
+#[ignore]
+#[test]
+fn rename_files_casing_only() -> anyhow::Result<()> {
+    let mut case = setup([("lib.py", "class Foo: ...")])?;
+
+    assert!(
+        resolve_module(case.db(), &ModuleName::new("lib").unwrap()).is_some(),
+        "Expected `lib` module to exist."
+    );
+    assert_eq!(
+        resolve_module(case.db(), &ModuleName::new("Lib").unwrap()),
+        None,
+        "Expected `Lib` module not to exist"
+    );
+
+    // Now rename `lib.py` to `Lib.py`
+    if case.db().system().case_sensitivity().is_case_sensitive() {
+        std::fs::rename(
+            case.project_path("lib.py").as_std_path(),
+            case.project_path("Lib.py").as_std_path(),
+        )
+        .context("Failed to rename `lib.py` to `Lib.py`")?;
+    } else {
+        // On case-insensitive file systems, renaming a file to a different casing is a no-op.
+        // Rename to a different name first
+        std::fs::rename(
+            case.project_path("lib.py").as_std_path(),
+            case.project_path("temp.py").as_std_path(),
+        )
+        .context("Failed to rename `lib.py` to `temp.py`")?;
+
+        std::fs::rename(
+            case.project_path("temp.py").as_std_path(),
+            case.project_path("Lib.py").as_std_path(),
+        )
+        .context("Failed to rename `temp.py` to `Lib.py`")?;
+    }
+
+    let changes = case.stop_watch(event_for_file("Lib.py"));
+    case.apply_changes(changes);
+
+    // Resolving `lib` should now fail but `Lib` should now succeed
+    assert_eq!(
+        resolve_module(case.db(), &ModuleName::new("lib").unwrap()),
+        None,
+        "Expected `lib` module to no longer exist."
+    );
+
+    assert!(
+        resolve_module(case.db(), &ModuleName::new("Lib").unwrap()).is_some(),
+        "Expected `Lib` module to exist"
+    );
+
+    Ok(())
+}

crates/red_knot_python_semantic/resources/mdtest/import/basic.md

@@ -140,3 +140,27 @@ import b.foo  # error: [unresolved-import] "Cannot resolve import `b.foo`"
 
 ```py
 ```
+
+## Long paths
+
+It's unlikely that a single module component is as long as in this example, but Windows treats paths
+that are longer than 200 and something specially. This test ensures that Red Knot can handle those
+paths gracefully.
+
+```toml
+system = "os"
+```
+
+`AveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPath/__init__.py`:
+
+```py
+class Foo: ...
+```
+
+```py
+from AveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPathAveryLongPath import (
+    Foo,
+)
+
+reveal_type(Foo())  # revealed: Foo
+```

crates/red_knot_python_semantic/resources/mdtest/import/case_sensitive.md

@@ -1,10 +1,7 @@
 # Case Sensitive Imports
 
 ```toml
-# TODO: This test should use the real file system instead of the memory file system.
-# but we can't change the file system yet because the tests would then start failing for 
-# case-insensitive file systems.
-#system = "os"
+system = "os"
 ```
 
 Python's import system is case-sensitive even on case-insensitive file system. This means, importing

crates/red_knot_python_semantic/src/module_resolver/path.rs

@@ -63,6 +63,10 @@ impl ModulePath {
         self.relative_path.pop()
     }
 
+    pub(super) fn search_path(&self) -> &SearchPath {
+        &self.search_path
+    }
+
     #[must_use]
     pub(super) fn is_directory(&self, resolver: &ResolverContext) -> bool {
         let ModulePath {

crates/red_knot_python_semantic/src/module_resolver/resolver.rs

@@ -604,14 +604,29 @@ fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, File, Mod
 /// resolving modules.
 fn resolve_file_module(module: &ModulePath, resolver_state: &ResolverContext) -> Option<File> {
     // Stubs have precedence over source files
-    module
+    let file = module
         .with_pyi_extension()
         .to_file(resolver_state)
         .or_else(|| {
             module
                 .with_py_extension()
                 .and_then(|path| path.to_file(resolver_state))
-        })
+        })?;
+
+    // For system files, test if the path has the correct casing.
+    // We can skip this step for vendored files or virtual files because
+    // those file systems are case sensitive (we wouldn't get to this point).
+    if let Some(path) = file.path(resolver_state.db).as_system_path() {
+        let system = resolver_state.db.system();
+        if !system.case_sensitivity().is_case_sensitive()
+            && !system
+                .path_exists_case_sensitive(path, module.search_path().as_system_path().unwrap())
+        {
+            return None;
+        }
+    }
+
+    Some(file)
 }
 
 fn resolve_package<'a, 'db, I>(
@@ -1842,4 +1857,72 @@ not_a_directory
         let a_module = resolve_module(&db, &a_module_name).unwrap();
         assert_eq!(a_module.file().path(&db), &system_site_packages_location);
     }
+
+    #[test]
+    #[cfg(unix)]
+    fn case_sensitive_resolution_with_symlinked_directory() -> anyhow::Result<()> {
+        use anyhow::Context;
+        use ruff_db::system::OsSystem;
+
+        let temp_dir = tempfile::TempDir::new()?;
+        let root = SystemPathBuf::from_path_buf(
+            temp_dir
+                .path()
+                .canonicalize()
+                .context("Failed to canonicalized path")?,
+        )
+        .expect("UTF8 path for temp dir");
+
+        let mut db = TestDb::new();
+
+        let src = root.join("src");
+        let a_package_target = root.join("a-package");
+        let a_src = src.join("a");
+
+        db.use_system(OsSystem::new(&root));
+
+        db.write_file(
+            a_package_target.join("__init__.py"),
+            "class Foo: x: int = 4",
+        )
+        .context("Failed to write `a-package/__init__.py`")?;
+
+        db.write_file(src.join("main.py"), "print('Hy')")
+            .context("Failed to write `main.py`")?;
+
+        // The symlink triggers the slow-path in the `OsSystem`'s `exists_path_case_sensitive`
+        // code because canonicalizing the path for `a/__init__.py` results in `a-package/__init__.py`
+        std::os::unix::fs::symlink(a_package_target.as_std_path(), a_src.as_std_path())
+            .context("Failed to symlink `src/a` to `a-package`")?;
+
+        Program::from_settings(
+            &db,
+            ProgramSettings {
+                python_version: PythonVersion::default(),
+                python_platform: PythonPlatform::default(),
+                search_paths: SearchPathSettings {
+                    extra_paths: vec![],
+                    src_roots: vec![src],
+                    custom_typeshed: None,
+                    python_path: PythonPath::KnownSitePackages(vec![]),
+                },
+            },
+        )
+        .expect("Valid program settings");
+
+        // Now try to resolve the module `A` (note the capital `A` instead of `a`).
+        let a_module_name = ModuleName::new_static("A").unwrap();
+        assert_eq!(resolve_module(&db, &a_module_name), None);
+
+        // Now lookup the same module using the lowercase `a` and it should resolve to the file in the system site-packages
+        let a_module_name = ModuleName::new_static("a").unwrap();
+        let a_module = resolve_module(&db, &a_module_name).expect("a.py to resolve");
+        assert!(a_module
+            .file()
+            .path(&db)
+            .as_str()
+            .ends_with("src/a/__init__.py"),);
+
+        Ok(())
+    }
 }

crates/red_knot_server/src/system.rs

@@ -7,8 +7,8 @@ use lsp_types::Url;
 use ruff_db::file_revision::FileRevision;
 use ruff_db::system::walk_directory::WalkDirectoryBuilder;
 use ruff_db::system::{
-    DirectoryEntry, FileType, GlobError, Metadata, OsSystem, PatternError, Result, System,
-    SystemPath, SystemPathBuf, SystemVirtualPath, SystemVirtualPathBuf,
+    CaseSensitivity, DirectoryEntry, FileType, GlobError, Metadata, OsSystem, PatternError, Result,
+    System, SystemPath, SystemPathBuf, SystemVirtualPath, SystemVirtualPathBuf,
 };
 use ruff_notebook::{Notebook, NotebookError};
 
@@ -136,6 +136,10 @@ impl System for LSPSystem {
         self.os_system.canonicalize_path(path)
     }
 
+    fn path_exists_case_sensitive(&self, path: &SystemPath, prefix: &SystemPath) -> bool {
+        self.os_system.path_exists_case_sensitive(path, prefix)
+    }
+
     fn read_to_string(&self, path: &SystemPath) -> Result<String> {
         let document = self.system_path_to_document_ref(path)?;
 
@@ -219,6 +223,10 @@ impl System for LSPSystem {
     fn as_any_mut(&mut self) -> &mut dyn Any {
         self
     }
+
+    fn case_sensitivity(&self) -> CaseSensitivity {
+        self.os_system.case_sensitivity()
+    }
 }
 
 fn not_a_text_document(path: impl Display) -> std::io::Error {

crates/red_knot_test/src/db.rs

@@ -3,8 +3,8 @@ use red_knot_python_semantic::lint::{LintRegistry, RuleSelection};
 use red_knot_python_semantic::{default_lint_registry, Db as SemanticDb};
 use ruff_db::files::{File, Files};
 use ruff_db::system::{
-    DbWithWritableSystem, InMemorySystem, OsSystem, System, SystemPath, SystemPathBuf,
-    WritableSystem,
+    CaseSensitivity, DbWithWritableSystem, InMemorySystem, OsSystem, System, SystemPath,
+    SystemPathBuf, WritableSystem,
 };
 use ruff_db::vendored::VendoredFileSystem;
 use ruff_db::{Db as SourceDb, Upcast};
@@ -211,6 +211,15 @@ impl System for MdtestSystem {
         self.as_system().read_virtual_path_to_notebook(path)
     }
 
+    fn path_exists_case_sensitive(&self, path: &SystemPath, prefix: &SystemPath) -> bool {
+        self.as_system()
+            .path_exists_case_sensitive(&self.normalize_path(path), &self.normalize_path(prefix))
+    }
+
+    fn case_sensitivity(&self) -> CaseSensitivity {
+        self.as_system().case_sensitivity()
+    }
+
     fn current_directory(&self) -> &SystemPath {
         self.as_system().current_directory()
     }

crates/red_knot_test/src/lib.rs

@@ -113,7 +113,9 @@ fn run_test(
                 .canonicalize()
                 .expect("Canonicalizing to succeed");
             let root_path = SystemPathBuf::from_path_buf(root_path)
-                .expect("Temp directory to be a valid UTF8 path");
+                .expect("Temp directory to be a valid UTF8 path")
+                .simplified()
+                .to_path_buf();
 
             db.use_os_system_with_temp_dir(root_path, dir);
         }

crates/red_knot_wasm/src/lib.rs

@@ -11,8 +11,8 @@ use ruff_db::diagnostic::{DisplayDiagnosticConfig, OldDiagnosticTrait};
 use ruff_db::files::{system_path_to_file, File};
 use ruff_db::system::walk_directory::WalkDirectoryBuilder;
 use ruff_db::system::{
-    DirectoryEntry, GlobError, MemoryFileSystem, Metadata, PatternError, System, SystemPath,
-    SystemPathBuf, SystemVirtualPath,
+    CaseSensitivity, DirectoryEntry, GlobError, MemoryFileSystem, Metadata, PatternError, System,
+    SystemPath, SystemPathBuf, SystemVirtualPath,
 };
 use ruff_notebook::Notebook;
 
@@ -260,6 +260,14 @@ impl System for WasmSystem {
         Err(ruff_notebook::NotebookError::Io(not_found()))
     }
 
+    fn path_exists_case_sensitive(&self, path: &SystemPath, _prefix: &SystemPath) -> bool {
+        self.path_exists(path)
+    }
+
+    fn case_sensitivity(&self) -> CaseSensitivity {
+        CaseSensitivity::CaseSensitive
+    }
+
     fn current_directory(&self) -> &SystemPath {
         self.fs.current_directory()
     }