From 48f906e06c73fc3ee01dcf3ba312e6a780f5a1cb Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 6 Mar 2025 09:19:23 +0000 Subject: [PATCH] Add tests for case-sensitive module resolution (#16517) ## 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. --- .../resources/mdtest/import/case_sensitive.md | 126 ++++++++++++++++++ crates/red_knot_test/src/config.rs | 14 +- crates/red_knot_test/src/lib.rs | 14 +- crates/red_knot_test/src/parser.rs | 12 +- 4 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 crates/red_knot_python_semantic/resources/mdtest/import/case_sensitive.md diff --git a/crates/red_knot_python_semantic/resources/mdtest/import/case_sensitive.md b/crates/red_knot_python_semantic/resources/mdtest/import/case_sensitive.md new file mode 100644 index 0000000000000..5ce8c1a11911f --- /dev/null +++ b/crates/red_knot_python_semantic/resources/mdtest/import/case_sensitive.md @@ -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 +``` diff --git a/crates/red_knot_test/src/config.rs b/crates/red_knot_test/src/config.rs index ab24b4b843cd2..842e108f4e5cb 100644 --- a/crates/red_knot_test/src/config.rs +++ b/crates/red_knot_test/src/config.rs @@ -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; @@ -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)] @@ -53,7 +60,10 @@ pub(crate) struct Environment { pub(crate) python_platform: Option, /// Path to a custom typeshed directory. - pub(crate) typeshed: Option, + pub(crate) typeshed: Option, + + /// Additional search paths to consider when resolving modules. + pub(crate) extra_paths: Option>, } #[derive(Deserialize, Debug, Clone)] diff --git a/crates/red_knot_test/src/lib.rs b/crates/red_knot_test/src/lib.rs index bafc8e65fef0c..ef5e40953df9a 100644 --- a/crates/red_knot_test/src/lib.rs +++ b/crates/red_knot_test/src/lib.rs @@ -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; @@ -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; @@ -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); @@ -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![]), }, diff --git a/crates/red_knot_test/src/parser.rs b/crates/red_knot_test/src/parser.rs index d8cbc8a1ba9ff..2f5f3d563541b 100644 --- a/crates/red_knot_test/src/parser.rs +++ b/crates/red_knot_test/src/parser.rs @@ -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) @@ -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"