Skip to content

Commit 8664842

Browse files
authored
[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 #20603 (comment). 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.
1 parent 0092794 commit 8664842

File tree

2 files changed

+55
-17
lines changed

2 files changed

+55
-17
lines changed

crates/ty/tests/cli/python_environment.rs

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,40 @@ fn config_file_annotation_showing_where_python_version_set_typing_error() -> any
188188
Ok(())
189189
}
190190

191+
/// If `.` and `./src` are both registered as first-party search paths,
192+
/// the `./src` directory should take precedence for module resolution,
193+
/// because it is relative to `.`.
194+
#[test]
195+
fn src_subdirectory_takes_precedence_over_repo_root() -> anyhow::Result<()> {
196+
let case = CliTest::with_files([(
197+
"src/package/__init__.py",
198+
"from . import nonexistent_submodule",
199+
)])?;
200+
201+
// If `./src` didn't take priority over `.` here, we would report
202+
// "Module `src.package` has no member `nonexistent_submodule`"
203+
// instead of "Module `package` has no member `nonexistent_submodule`".
204+
assert_cmd_snapshot!(case.command(), @r"
205+
success: false
206+
exit_code: 1
207+
----- stdout -----
208+
error[unresolved-import]: Module `package` has no member `nonexistent_submodule`
209+
--> src/package/__init__.py:1:15
210+
|
211+
1 | from . import nonexistent_submodule
212+
| ^^^^^^^^^^^^^^^^^^^^^
213+
|
214+
info: rule `unresolved-import` is enabled by default
215+
216+
Found 1 diagnostic
217+
218+
----- stderr -----
219+
WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors.
220+
");
221+
222+
Ok(())
223+
}
224+
191225
/// This tests that, even if no Python *version* has been specified on the CLI or in a config file,
192226
/// ty is still able to infer the Python version from a `--python` argument on the CLI,
193227
/// *even if* the `--python` argument points to a system installation.
@@ -1738,8 +1772,8 @@ fn default_root_tests_package() -> anyhow::Result<()> {
17381772
5 | print(f"{foo} {bar}")
17391773
|
17401774
info: Searched in the following paths during module resolution:
1741-
info: 1. <temp_dir>/ (first-party code)
1742-
info: 2. <temp_dir>/src (first-party code)
1775+
info: 1. <temp_dir>/src (first-party code)
1776+
info: 2. <temp_dir>/ (first-party code)
17431777
info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty)
17441778
info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment
17451779
info: rule `unresolved-import` is enabled by default
@@ -1814,8 +1848,8 @@ fn default_root_python_package() -> anyhow::Result<()> {
18141848
5 | print(f"{foo} {bar}")
18151849
|
18161850
info: Searched in the following paths during module resolution:
1817-
info: 1. <temp_dir>/ (first-party code)
1818-
info: 2. <temp_dir>/src (first-party code)
1851+
info: 1. <temp_dir>/src (first-party code)
1852+
info: 2. <temp_dir>/ (first-party code)
18191853
info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty)
18201854
info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment
18211855
info: rule `unresolved-import` is enabled by default
@@ -1861,8 +1895,8 @@ fn default_root_python_package_pyi() -> anyhow::Result<()> {
18611895
5 | print(f"{foo} {bar}")
18621896
|
18631897
info: Searched in the following paths during module resolution:
1864-
info: 1. <temp_dir>/ (first-party code)
1865-
info: 2. <temp_dir>/src (first-party code)
1898+
info: 1. <temp_dir>/src (first-party code)
1899+
info: 2. <temp_dir>/ (first-party code)
18661900
info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty)
18671901
info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment
18681902
info: rule `unresolved-import` is enabled by default
@@ -1902,8 +1936,8 @@ fn pythonpath_is_respected() -> anyhow::Result<()> {
19021936
3 | print(f"{baz.it}")
19031937
|
19041938
info: Searched in the following paths during module resolution:
1905-
info: 1. <temp_dir>/ (first-party code)
1906-
info: 2. <temp_dir>/src (first-party code)
1939+
info: 1. <temp_dir>/src (first-party code)
1940+
info: 2. <temp_dir>/ (first-party code)
19071941
info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty)
19081942
info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment
19091943
info: rule `unresolved-import` is enabled by default
@@ -1959,8 +1993,8 @@ fn pythonpath_multiple_dirs_is_respected() -> anyhow::Result<()> {
19591993
3 | import foo
19601994
|
19611995
info: Searched in the following paths during module resolution:
1962-
info: 1. <temp_dir>/ (first-party code)
1963-
info: 2. <temp_dir>/src (first-party code)
1996+
info: 1. <temp_dir>/src (first-party code)
1997+
info: 2. <temp_dir>/ (first-party code)
19641998
info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty)
19651999
info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment
19662000
info: rule `unresolved-import` is enabled by default
@@ -1975,8 +2009,8 @@ fn pythonpath_multiple_dirs_is_respected() -> anyhow::Result<()> {
19752009
5 | print(f"{baz.it}")
19762010
|
19772011
info: Searched in the following paths during module resolution:
1978-
info: 1. <temp_dir>/ (first-party code)
1979-
info: 2. <temp_dir>/src (first-party code)
2012+
info: 1. <temp_dir>/src (first-party code)
2013+
info: 2. <temp_dir>/ (first-party code)
19802014
info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty)
19812015
info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment
19822016
info: rule `unresolved-import` is enabled by default

crates/ty_project/src/metadata/options.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -237,28 +237,28 @@ impl Options {
237237
.map(|root| root.absolute(project_root, system))
238238
.collect()
239239
} else {
240+
let mut roots = vec![];
240241
let src = project_root.join("src");
241242

242-
let mut roots = if system.is_directory(&src) {
243+
if system.is_directory(&src) {
243244
// Default to `src` and the project root if `src` exists and the root hasn't been specified.
244245
// This corresponds to the `src-layout`
245246
tracing::debug!(
246247
"Including `.` and `./src` in `environment.root` because a `./src` directory exists"
247248
);
248-
vec![project_root.to_path_buf(), src]
249+
roots.push(src);
249250
} else if system.is_directory(&project_root.join(project_name).join(project_name)) {
250251
// `src-layout` but when the folder isn't called `src` but has the same name as the project.
251252
// For example, the "src" folder for `psycopg` is called `psycopg` and the python files are in `psycopg/psycopg/_adapters_map.py`
252253
tracing::debug!(
253254
"Including `.` and `/{project_name}` in `environment.root` because a `./{project_name}/{project_name}` directory exists"
254255
);
255256

256-
vec![project_root.to_path_buf(), project_root.join(project_name)]
257+
roots.push(project_root.join(project_name));
257258
} else {
258259
// Default to a [flat project structure](https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/).
259260
tracing::debug!("Including `.` in `environment.root`");
260-
vec![project_root.to_path_buf()]
261-
};
261+
}
262262

263263
let python = project_root.join("python");
264264
if system.is_directory(&python)
@@ -293,6 +293,10 @@ impl Options {
293293
roots.push(tests_dir);
294294
}
295295

296+
// The project root should always be included, and should always come
297+
// after any subdirectories such as `./src`, `./tests` and/or `./python`.
298+
roots.push(project_root.to_path_buf());
299+
296300
roots
297301
};
298302

0 commit comments

Comments
 (0)