[red-knot] Case sensitive module resolver

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()
     }

crates/ruff_db/src/source.rs

@@ -159,7 +159,7 @@ pub enum SourceTextError {
 /// Computes the [`LineIndex`] for `file`.
 #[salsa::tracked]
 pub fn line_index(db: &dyn Db, file: File) -> LineIndex {
-    let _span = tracing::trace_span!("line_index", file = ?file).entered();
+    let _span = tracing::trace_span!("line_index", file = ?file.path(db)).entered();
 
     let source = source_text(db, file);