Skip to content

Commit

Permalink
Add tests for case-sensitive module resolution (#16517)
Browse files Browse the repository at this point in the history
## Summary

Python's module resolver is case sensitive. 

This PR adds mdtests that assert that our module resolution is case
sensitive.

The tests currently all pass because our in memory file system is case
sensitive.
I'll add support for using the real file system to the mdtest framework
in a separate PR.

This PR also adds support for specifying extra search paths to the
mdtest framework.

## Test Plan
The tests fail when running them using the real file system.
  • Loading branch information
MichaReiser authored Mar 6, 2025
1 parent ebd172e commit 48f906e
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
# Case Sensitive Imports

TODO: This test should use the real file system instead of the memory file system.

Python's import system is case-sensitive even on case-insensitive file system. This means, importing
a module `a` should fail if the file in the search paths is named `A.py`. See
[PEP 235](https://peps.python.org/pep-0235/).

## Correct casing

Importing a module where the name matches the file name's casing should succeed.

`a.py`:

```py
class Foo:
x: int = 1
```

```python
from a import Foo

reveal_type(Foo().x) # revealed: int
```

## Incorrect casing

Importing a module where the name does not match the file name's casing should fail.

`A.py`:

```py
class Foo:
x: int = 1
```

```python
# error: [unresolved-import]
from a import Foo
```

## Multiple search paths with different cased modules

The resolved module is the first matching the file name's casing but Python falls back to later
search paths if the file name's casing does not match.

```toml
[environment]
extra-paths = ["/search-1", "/search-2"]
```

`/search-1/A.py`:

```py
class Foo:
x: int = 1
```

`/search-2/a.py`:

```py
class Bar:
x: str = "test"
```

```python
from A import Foo
from a import Bar

reveal_type(Foo().x) # revealed: int
reveal_type(Bar().x) # revealed: str
```

## Intermediate segments

`db/__init__.py`:

```py
```

`db/a.py`:

```py
class Foo:
x: int = 1
```

`correctly_cased.py`:

```python
from db.a import Foo

reveal_type(Foo().x) # revealed: int
```

Imports where some segments are incorrectly cased should fail.

`incorrectly_cased.py`:

```python
# error: [unresolved-import]
from DB.a import Foo

# error: [unresolved-import]
from DB.A import Foo

# error: [unresolved-import]
from db.A import Foo
```

## Incorrect extension casing

The extension of imported python modules must be `.py` or `.pyi` but not `.PY` or `Py` or any
variant where some characters are uppercase.

`a.PY`:

```py
class Foo:
x: int = 1
```

```python
# error: [unresolved-import]
from a import Foo
```
14 changes: 12 additions & 2 deletions crates/red_knot_test/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use anyhow::Context;
use red_knot_python_semantic::PythonPlatform;
use ruff_db::system::{SystemPath, SystemPathBuf};
use ruff_python_ast::PythonVersion;
use serde::Deserialize;

Expand All @@ -36,11 +37,17 @@ impl MarkdownTestConfig {
.and_then(|env| env.python_platform.clone())
}

pub(crate) fn typeshed(&self) -> Option<&str> {
pub(crate) fn typeshed(&self) -> Option<&SystemPath> {
self.environment
.as_ref()
.and_then(|env| env.typeshed.as_deref())
}

pub(crate) fn extra_paths(&self) -> Option<&[SystemPathBuf]> {
self.environment
.as_ref()
.and_then(|env| env.extra_paths.as_deref())
}
}

#[derive(Deserialize, Debug, Default, Clone)]
Expand All @@ -53,7 +60,10 @@ pub(crate) struct Environment {
pub(crate) python_platform: Option<PythonPlatform>,

/// Path to a custom typeshed directory.
pub(crate) typeshed: Option<String>,
pub(crate) typeshed: Option<SystemPathBuf>,

/// Additional search paths to consider when resolving modules.
pub(crate) extra_paths: Option<Vec<SystemPathBuf>>,
}

#[derive(Deserialize, Debug, Clone)]
Expand Down
14 changes: 9 additions & 5 deletions crates/red_knot_test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use ruff_db::diagnostic::{DisplayDiagnosticConfig, OldDiagnosticTrait, OldParseD
use ruff_db::files::{system_path_to_file, File, Files};
use ruff_db::panic::catch_unwind;
use ruff_db::parsed::parsed_module;
use ruff_db::system::{DbWithTestSystem, SystemPathBuf};
use ruff_db::system::{DbWithTestSystem, SystemPath, SystemPathBuf};
use ruff_db::testing::{setup_logging, setup_logging_with_filter};
use ruff_source_file::{LineIndex, OneIndexed};
use std::fmt::Write;
Expand Down Expand Up @@ -106,7 +106,7 @@ fn run_test(
) -> Result<(), Failures> {
let project_root = db.project_root().to_path_buf();
let src_path = SystemPathBuf::from("/src");
let custom_typeshed_path = test.configuration().typeshed().map(SystemPathBuf::from);
let custom_typeshed_path = test.configuration().typeshed().map(SystemPath::to_path_buf);
let mut typeshed_files = vec![];
let mut has_custom_versions_file = false;

Expand All @@ -118,8 +118,8 @@ fn run_test(
}

assert!(
matches!(embedded.lang, "py" | "pyi" | "text"),
"Supported file types are: py, pyi, text"
matches!(embedded.lang, "py" | "pyi" | "python" | "text"),
"Supported file types are: py (or python), pyi, text, and ignore"
);

let full_path = embedded.full_path(&project_root);
Expand Down Expand Up @@ -178,7 +178,11 @@ fn run_test(
python_platform: test.configuration().python_platform().unwrap_or_default(),
search_paths: SearchPathSettings {
src_roots: vec![src_path],
extra_paths: vec![],
extra_paths: test
.configuration()
.extra_paths()
.unwrap_or_default()
.to_vec(),
custom_typeshed: custom_typeshed_path,
python_path: PythonPath::KnownSitePackages(vec![]),
},
Expand Down
12 changes: 9 additions & 3 deletions crates/red_knot_test/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,10 @@ impl EmbeddedFile<'_> {
self.path.as_str()
}

/// Returns the full path using unix file-path convention.
pub(crate) fn full_path(&self, project_root: &SystemPath) -> SystemPathBuf {
// Don't use `SystemPath::absolute` here because it's platform dependent
// and we want to use unix file-path convention.
let relative_path = self.relative_path();
if relative_path.starts_with('/') {
SystemPathBuf::from(relative_path)
Expand Down Expand Up @@ -606,10 +609,13 @@ impl<'s> Parser<'s> {
}

if let Some(explicit_path) = self.explicit_path {
if !lang.is_empty()
let expected_extension = if lang == "python" { "py" } else { lang };

if !expected_extension.is_empty()
&& lang != "text"
&& explicit_path.contains('.')
&& !explicit_path.ends_with(&format!(".{lang}"))
&& !SystemPath::new(explicit_path)
.extension()
.is_none_or(|extension| extension.eq_ignore_ascii_case(expected_extension))
{
bail!(
"File extension of test file path `{explicit_path}` in test `{test_name}` does not match language specified `{lang}` of code block"
Expand Down

0 comments on commit 48f906e

Please sign in to comment.