Skip to content

Commit dbde770

Browse files
committed
Initial version of case-sensitive module resolver
1 parent edcb9e6 commit dbde770

File tree

10 files changed

+160
-8
lines changed

10 files changed

+160
-8
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# TODO: This test should use the real file system instead of the memory file system.
55
# but we can't change the file system yet because the tests would then start failing for
66
# case-insensitive file systems.
7-
#system = "os"
7+
system = "os"
88
```
99

1010
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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ impl ModulePath {
6363
self.relative_path.pop()
6464
}
6565

66+
pub(super) fn search_path(&self) -> &SearchPath {
67+
&self.search_path
68+
}
69+
6670
#[must_use]
6771
pub(super) fn is_directory(&self, resolver: &ResolverContext) -> bool {
6872
let ModulePath {

crates/red_knot_python_semantic/src/module_resolver/resolver.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -604,14 +604,30 @@ fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, File, Mod
604604
/// resolving modules.
605605
fn resolve_file_module(module: &ModulePath, resolver_state: &ResolverContext) -> Option<File> {
606606
// Stubs have precedence over source files
607-
module
607+
let file = module
608608
.with_pyi_extension()
609609
.to_file(resolver_state)
610610
.or_else(|| {
611611
module
612612
.with_py_extension()
613613
.and_then(|path| path.to_file(resolver_state))
614-
})
614+
})?;
615+
616+
// For system files, test if the path has the correct casing.
617+
// We can skip this step for vendored files or virtual files because
618+
// those file systems are case sensitive (we wouldn't get to this point).
619+
if let Some(path) = file.path(resolver_state.db).as_system_path() {
620+
if !resolver_state
621+
.db
622+
.system()
623+
.path_exists_case_sensitive(path, module.search_path().as_system_path().unwrap())
624+
.unwrap_or(true)
625+
{
626+
return None;
627+
}
628+
}
629+
630+
Some(file)
615631
}
616632

617633
fn resolve_package<'a, 'db, I>(

crates/red_knot_server/src/system.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ impl System for LSPSystem {
136136
self.os_system.canonicalize_path(path)
137137
}
138138

139+
fn path_exists_case_sensitive(&self, path: &SystemPath, prefix: &SystemPath) -> Result<bool> {
140+
self.os_system.path_exists_case_sensitive(path, prefix)
141+
}
142+
139143
fn read_to_string(&self, path: &SystemPath) -> Result<String> {
140144
let document = self.system_path_to_document_ref(path)?;
141145

crates/red_knot_test/src/db.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,15 @@ impl System for MdtestSystem {
210210
self.as_system().read_virtual_path_to_notebook(path)
211211
}
212212

213+
fn path_exists_case_sensitive(
214+
&self,
215+
path: &SystemPath,
216+
prefix: &SystemPath,
217+
) -> ruff_db::system::Result<bool> {
218+
self.as_system()
219+
.path_exists_case_sensitive(&self.normalize_path(path), &self.normalize_path(prefix))
220+
}
221+
213222
fn current_directory(&self) -> &SystemPath {
214223
self.as_system().current_directory()
215224
}

crates/red_knot_wasm/src/lib.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,14 @@ impl System for WasmSystem {
260260
Err(ruff_notebook::NotebookError::Io(not_found()))
261261
}
262262

263+
fn path_exists_case_sensitive(
264+
&self,
265+
path: &SystemPath,
266+
_prefix: &SystemPath,
267+
) -> ruff_db::system::Result<bool> {
268+
Ok(self.path_exists(path))
269+
}
270+
263271
fn current_directory(&self) -> &SystemPath {
264272
self.fs.current_directory()
265273
}

crates/ruff_db/src/source.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ pub enum SourceTextError {
159159
/// Computes the [`LineIndex`] for `file`.
160160
#[salsa::tracked]
161161
pub fn line_index(db: &dyn Db, file: File) -> LineIndex {
162-
let _span = tracing::trace_span!("line_index", file = ?file).entered();
162+
let _span = tracing::trace_span!("line_index", file = ?file.path(db)).entered();
163163

164164
let source = source_text(db, file);
165165

crates/ruff_db/src/system.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ pub trait System: Debug {
8989
self.path_metadata(path).is_ok()
9090
}
9191

92+
fn path_exists_case_sensitive(&self, path: &SystemPath, prefix: &SystemPath) -> Result<bool>;
93+
9294
/// Returns `true` if `path` exists and is a directory.
9395
fn is_directory(&self, path: &SystemPath) -> bool {
9496
self.path_metadata(path)

crates/ruff_db/src/system/os.rs

Lines changed: 103 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
use std::sync::Arc;
2-
use std::{any::Any, path::PathBuf};
3-
41
use filetime::FileTime;
5-
62
use ruff_notebook::{Notebook, NotebookError};
3+
use rustc_hash::FxHashSet;
4+
use std::panic::RefUnwindSafe;
5+
use std::sync::Arc;
6+
use std::{any::Any, path::PathBuf};
77

88
use crate::system::{
99
DirectoryEntry, FileType, GlobError, GlobErrorKind, Metadata, Result, System, SystemPath,
@@ -25,6 +25,8 @@ pub struct OsSystem {
2525
struct OsSystemInner {
2626
cwd: SystemPathBuf,
2727

28+
real_case_cache: RealCaseCache,
29+
2830
/// Overrides the user's configuration directory for testing.
2931
/// This is an `Option<Option<..>>` to allow setting an override of `None`.
3032
#[cfg(feature = "testing")]
@@ -102,6 +104,27 @@ impl System for OsSystem {
102104
path.as_std_path().exists()
103105
}
104106

107+
fn path_exists_case_sensitive(&self, path: &SystemPath, prefix: &SystemPath) -> Result<bool> {
108+
// Decide what to do if prefix isn't a prefix
109+
debug_assert!(
110+
path.strip_prefix(prefix).is_ok(),
111+
"prefix `{prefix}` should be a prefix of `{path}`"
112+
);
113+
114+
// Iterate over the sub-paths up to prefix and check if they match the casing as on disk.
115+
for ancestor in path.ancestors() {
116+
if ancestor == prefix {
117+
break;
118+
}
119+
120+
if !self.inner.real_case_cache.has_name_case(ancestor)? {
121+
return Ok(false);
122+
}
123+
}
124+
125+
Ok(true)
126+
}
127+
105128
fn current_directory(&self) -> &SystemPath {
106129
&self.inner.cwd
107130
}
@@ -201,6 +224,82 @@ impl WritableSystem for OsSystem {
201224
}
202225
}
203226

227+
#[derive(Debug, Default)]
228+
struct RealCaseCache {
229+
by_lower_case: dashmap::DashMap<SystemPathBuf, ListedDirectory>,
230+
}
231+
232+
impl RealCaseCache {
233+
fn has_name_case(&self, path: &SystemPath) -> Result<bool> {
234+
// TODO: Skip if the FS is known to be case-sensitive.
235+
// TODO: Consider using `GetFinalPathNameByHandleW` on windows
236+
// TODO: Consider using `fcntl(F_GETPATH)` on macOS (https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fcntl.2.html)
237+
238+
let Some(parent) = path.parent() else {
239+
// TODO: Decide what to return for the root path
240+
return Ok(true);
241+
};
242+
243+
// TODO: decide what to return for the root path or files ending in `..`
244+
let Some(file_name) = path.file_name() else {
245+
return Ok(false);
246+
};
247+
248+
let lower_case_path = SystemPathBuf::from(parent.as_str().to_lowercase());
249+
let last_modification_time =
250+
FileTime::from_last_modification_time(&parent.as_std_path().metadata()?);
251+
252+
let entry = self.by_lower_case.entry(lower_case_path);
253+
254+
if let dashmap::Entry::Occupied(entry) = &entry {
255+
// Only do a cached lookup if the directory hasn't changed.
256+
if entry.get().last_modification_time == last_modification_time {
257+
tracing::trace!("Use cached 'real-case' entry for directory `{}`", parent);
258+
return Ok(entry.get().names.contains(file_name));
259+
}
260+
}
261+
262+
tracing::trace!(
263+
"Reading directory `{}` to determine the real casing of paths",
264+
parent
265+
);
266+
let start = std::time::Instant::now();
267+
let mut names = FxHashSet::default();
268+
269+
for entry in parent.as_std_path().read_dir()? {
270+
let Ok(entry) = entry else {
271+
continue;
272+
};
273+
274+
let Ok(name) = entry.file_name().into_string() else {
275+
continue;
276+
};
277+
278+
names.insert(name);
279+
}
280+
281+
let directory = entry.insert(ListedDirectory {
282+
last_modification_time,
283+
names,
284+
});
285+
286+
tracing::trace!(
287+
"Caching the real casing of `{parent}` took {:?}",
288+
start.elapsed()
289+
);
290+
291+
Ok(directory.names.contains(file_name))
292+
}
293+
}
294+
295+
impl RefUnwindSafe for RealCaseCache {}
296+
297+
#[derive(Debug, Eq, PartialEq)]
298+
struct ListedDirectory {
299+
last_modification_time: filetime::FileTime,
300+
names: FxHashSet<String>,
301+
}
302+
204303
#[derive(Debug)]
205304
struct OsDirectoryWalker;
206305

crates/ruff_db/src/system/test.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ impl System for TestSystem {
130130
fn as_any_mut(&mut self) -> &mut dyn std::any::Any {
131131
self
132132
}
133+
134+
fn path_exists_case_sensitive(&self, path: &SystemPath, prefix: &SystemPath) -> Result<bool> {
135+
self.system().path_exists_case_sensitive(path, prefix)
136+
}
133137
}
134138

135139
impl Default for TestSystem {
@@ -349,6 +353,12 @@ impl System for InMemorySystem {
349353
fn as_any_mut(&mut self) -> &mut dyn std::any::Any {
350354
self
351355
}
356+
357+
#[inline]
358+
fn path_exists_case_sensitive(&self, path: &SystemPath, _prefix: &SystemPath) -> Result<bool> {
359+
// The memory file system is case-sensitive.
360+
Ok(self.path_exists(path))
361+
}
352362
}
353363

354364
impl WritableSystem for InMemorySystem {

0 commit comments

Comments
 (0)