From 8664842d00d846ac7fa1ff8b7ecf05ef67cd75af Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 29 Sep 2025 21:19:13 +0100 Subject: [PATCH] [ty] Ensure first-party search paths always appear in a sensible order (#20629) This PR ensures that we always put `./src` before `.` in our list of first-party search paths. This better emulates the fact that at runtime, the module name of a file `src/foo.py` would almost certainly be `foo` rather than `src.foo`. I wondered if fixing this might fix https://github.com/astral-sh/ruff/pull/20603#issuecomment-3345317444. It seems like that's not the case, but it also seems like it leads to better diagnostics because we report much more intuitive module names to the user in our error messages -- so, it's probably a good change anyway. --- crates/ty/tests/cli/python_environment.rs | 58 ++++++++++++++++++----- crates/ty_project/src/metadata/options.rs | 14 ++++-- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/crates/ty/tests/cli/python_environment.rs b/crates/ty/tests/cli/python_environment.rs index 062dbbef9b..483379368a 100644 --- a/crates/ty/tests/cli/python_environment.rs +++ b/crates/ty/tests/cli/python_environment.rs @@ -188,6 +188,40 @@ fn config_file_annotation_showing_where_python_version_set_typing_error() -> any Ok(()) } +/// If `.` and `./src` are both registered as first-party search paths, +/// the `./src` directory should take precedence for module resolution, +/// because it is relative to `.`. +#[test] +fn src_subdirectory_takes_precedence_over_repo_root() -> anyhow::Result<()> { + let case = CliTest::with_files([( + "src/package/__init__.py", + "from . import nonexistent_submodule", + )])?; + + // If `./src` didn't take priority over `.` here, we would report + // "Module `src.package` has no member `nonexistent_submodule`" + // instead of "Module `package` has no member `nonexistent_submodule`". + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + error[unresolved-import]: Module `package` has no member `nonexistent_submodule` + --> src/package/__init__.py:1:15 + | + 1 | from . import nonexistent_submodule + | ^^^^^^^^^^^^^^^^^^^^^ + | + info: rule `unresolved-import` is enabled by default + + Found 1 diagnostic + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + "); + + Ok(()) +} + /// This tests that, even if no Python *version* has been specified on the CLI or in a config file, /// ty is still able to infer the Python version from a `--python` argument on the CLI, /// *even if* the `--python` argument points to a system installation. @@ -1738,8 +1772,8 @@ fn default_root_tests_package() -> anyhow::Result<()> { 5 | print(f"{foo} {bar}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1814,8 +1848,8 @@ fn default_root_python_package() -> anyhow::Result<()> { 5 | print(f"{foo} {bar}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1861,8 +1895,8 @@ fn default_root_python_package_pyi() -> anyhow::Result<()> { 5 | print(f"{foo} {bar}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1902,8 +1936,8 @@ fn pythonpath_is_respected() -> anyhow::Result<()> { 3 | print(f"{baz.it}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1959,8 +1993,8 @@ fn pythonpath_multiple_dirs_is_respected() -> anyhow::Result<()> { 3 | import foo | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default @@ -1975,8 +2009,8 @@ fn pythonpath_multiple_dirs_is_respected() -> anyhow::Result<()> { 5 | print(f"{baz.it}") | info: Searched in the following paths during module resolution: - info: 1. / (first-party code) - info: 2. /src (first-party code) + info: 1. /src (first-party code) + info: 2. / (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index 9bcc0067c4..c0e0cfeb91 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -237,15 +237,16 @@ impl Options { .map(|root| root.absolute(project_root, system)) .collect() } else { + let mut roots = vec![]; let src = project_root.join("src"); - let mut roots = if system.is_directory(&src) { + if system.is_directory(&src) { // Default to `src` and the project root if `src` exists and the root hasn't been specified. // This corresponds to the `src-layout` tracing::debug!( "Including `.` and `./src` in `environment.root` because a `./src` directory exists" ); - vec![project_root.to_path_buf(), src] + roots.push(src); } else if system.is_directory(&project_root.join(project_name).join(project_name)) { // `src-layout` but when the folder isn't called `src` but has the same name as the project. // For example, the "src" folder for `psycopg` is called `psycopg` and the python files are in `psycopg/psycopg/_adapters_map.py` @@ -253,12 +254,11 @@ impl Options { "Including `.` and `/{project_name}` in `environment.root` because a `./{project_name}/{project_name}` directory exists" ); - vec![project_root.to_path_buf(), project_root.join(project_name)] + roots.push(project_root.join(project_name)); } else { // Default to a [flat project structure](https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/). tracing::debug!("Including `.` in `environment.root`"); - vec![project_root.to_path_buf()] - }; + } let python = project_root.join("python"); if system.is_directory(&python) @@ -293,6 +293,10 @@ impl Options { roots.push(tests_dir); } + // The project root should always be included, and should always come + // after any subdirectories such as `./src`, `./tests` and/or `./python`. + roots.push(project_root.to_path_buf()); + roots };