From 45ac30a4d762c9c564d45bb66df813666682e78e Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Wed, 3 Dec 2025 15:04:36 -0500 Subject: [PATCH] [ty] Teach `ty` the meaning of desperation (try ancestor `pyproject.toml`s as search-paths if module resolution fails) (#21745) ## Summary This makes an importing file a required argument to module resolution, and if the fast-path cached query fails to resolve the module, take the slow-path uncached (could be cached if we want) `desperately_resolve_module` which will walk up from the importing file until it finds a `pyproject.toml` (arbitrary decision, we could try every ancestor directory), at which point it takes one last desperate attempt to use that directory as a search-path. We do not continue walking up once we've found a `pyproject.toml` (arbitrary decision, we could keep going up). Running locally, this fixes every broken-for-workspace-reasons import in pyx's workspace! * Fixes https://github.com/astral-sh/ty/issues/1539 * Improves https://github.com/astral-sh/ty/issues/839 ## Test Plan The workspace tests see a huge improvement on most absolute imports. --- crates/ruff_graph/src/lib.rs | 2 +- crates/ruff_graph/src/resolver.rs | 27 +- crates/ty/tests/file_watching.rs | 70 ++-- crates/ty_ide/src/all_symbols.rs | 2 +- .../resources/mdtest/import/workspaces.md | 364 +++++++++--------- crates/ty_python_semantic/src/dunder_all.rs | 2 +- crates/ty_python_semantic/src/lib.rs | 4 +- .../src/module_resolver/mod.rs | 5 +- .../src/module_resolver/path.rs | 23 +- .../src/module_resolver/resolver.rs | 360 +++++++++++++---- crates/ty_python_semantic/src/place.rs | 10 +- .../src/semantic_index/builder.rs | 2 +- .../src/semantic_index/re_exports.rs | 4 +- .../ty_python_semantic/src/semantic_model.rs | 8 +- crates/ty_python_semantic/src/types.rs | 2 +- crates/ty_python_semantic/src/types/class.rs | 5 +- .../ty_python_semantic/src/types/function.rs | 2 +- .../src/types/ide_support.rs | 11 +- .../src/types/infer/builder.rs | 12 +- .../src/types/special_form.rs | 5 +- crates/ty_test/src/lib.rs | 22 +- 21 files changed, 614 insertions(+), 328 deletions(-) diff --git a/crates/ruff_graph/src/lib.rs b/crates/ruff_graph/src/lib.rs index 64647c8b17..0ada26454f 100644 --- a/crates/ruff_graph/src/lib.rs +++ b/crates/ruff_graph/src/lib.rs @@ -49,7 +49,7 @@ impl ModuleImports { // Resolve the imports. let mut resolved_imports = ModuleImports::default(); for import in imports { - for resolved in Resolver::new(db).resolve(import) { + for resolved in Resolver::new(db, path).resolve(import) { if let Some(path) = resolved.as_system_path() { resolved_imports.insert(path.to_path_buf()); } diff --git a/crates/ruff_graph/src/resolver.rs b/crates/ruff_graph/src/resolver.rs index f1f1589958..b942f81a9a 100644 --- a/crates/ruff_graph/src/resolver.rs +++ b/crates/ruff_graph/src/resolver.rs @@ -1,5 +1,9 @@ -use ruff_db::files::FilePath; -use ty_python_semantic::{ModuleName, resolve_module, resolve_real_module}; +use ruff_db::files::{File, FilePath, system_path_to_file}; +use ruff_db::system::SystemPath; +use ty_python_semantic::{ + ModuleName, resolve_module, resolve_module_confident, resolve_real_module, + resolve_real_module_confident, +}; use crate::ModuleDb; use crate::collector::CollectedImport; @@ -7,12 +11,15 @@ use crate::collector::CollectedImport; /// Collect all imports for a given Python file. pub(crate) struct Resolver<'a> { db: &'a ModuleDb, + file: Option, } impl<'a> Resolver<'a> { /// Initialize a [`Resolver`] with a given [`ModuleDb`]. - pub(crate) fn new(db: &'a ModuleDb) -> Self { - Self { db } + pub(crate) fn new(db: &'a ModuleDb, path: &SystemPath) -> Self { + // If we know the importing file we can potentially resolve more imports + let file = system_path_to_file(db, path).ok(); + Self { db, file } } /// Resolve the [`CollectedImport`] into a [`FilePath`]. @@ -70,13 +77,21 @@ impl<'a> Resolver<'a> { /// Resolves a module name to a module. pub(crate) fn resolve_module(&self, module_name: &ModuleName) -> Option<&'a FilePath> { - let module = resolve_module(self.db, module_name)?; + let module = if let Some(file) = self.file { + resolve_module(self.db, file, module_name)? + } else { + resolve_module_confident(self.db, module_name)? + }; Some(module.file(self.db)?.path(self.db)) } /// Resolves a module name to a module (stubs not allowed). fn resolve_real_module(&self, module_name: &ModuleName) -> Option<&'a FilePath> { - let module = resolve_real_module(self.db, module_name)?; + let module = if let Some(file) = self.file { + resolve_real_module(self.db, file, module_name)? + } else { + resolve_real_module_confident(self.db, module_name)? + }; Some(module.file(self.db)?.path(self.db)) } } diff --git a/crates/ty/tests/file_watching.rs b/crates/ty/tests/file_watching.rs index 5a738b5fd5..5bb80ca857 100644 --- a/crates/ty/tests/file_watching.rs +++ b/crates/ty/tests/file_watching.rs @@ -15,7 +15,7 @@ use ty_project::metadata::pyproject::{PyProject, Tool}; use ty_project::metadata::value::{RangedValue, RelativePathBuf}; use ty_project::watch::{ChangeEvent, ProjectWatcher, directory_watcher}; use ty_project::{Db, ProjectDatabase, ProjectMetadata}; -use ty_python_semantic::{Module, ModuleName, PythonPlatform, resolve_module}; +use ty_python_semantic::{Module, ModuleName, PythonPlatform, resolve_module_confident}; struct TestCase { db: ProjectDatabase, @@ -232,7 +232,8 @@ impl TestCase { } fn module<'c>(&'c self, name: &str) -> Module<'c> { - resolve_module(self.db(), &ModuleName::new(name).unwrap()).expect("module to be present") + resolve_module_confident(self.db(), &ModuleName::new(name).unwrap()) + .expect("module to be present") } fn sorted_submodule_names(&self, parent_module_name: &str) -> Vec { @@ -811,7 +812,8 @@ fn directory_moved_to_project() -> anyhow::Result<()> { .with_context(|| "Failed to create __init__.py")?; std::fs::write(a_original_path.as_std_path(), "").with_context(|| "Failed to create a.py")?; - let sub_a_module = resolve_module(case.db(), &ModuleName::new_static("sub.a").unwrap()); + let sub_a_module = + resolve_module_confident(case.db(), &ModuleName::new_static("sub.a").unwrap()); assert_eq!(sub_a_module, None); case.assert_indexed_project_files([bar]); @@ -832,7 +834,9 @@ fn directory_moved_to_project() -> anyhow::Result<()> { .expect("a.py to exist"); // `import sub.a` should now resolve - assert!(resolve_module(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_some()); + assert!( + resolve_module_confident(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_some() + ); case.assert_indexed_project_files([bar, init_file, a_file]); @@ -848,7 +852,9 @@ fn directory_moved_to_trash() -> anyhow::Result<()> { ])?; let bar = case.system_file(case.project_path("bar.py")).unwrap(); - assert!(resolve_module(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_some()); + assert!( + resolve_module_confident(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_some() + ); let sub_path = case.project_path("sub"); let init_file = case @@ -870,7 +876,9 @@ fn directory_moved_to_trash() -> anyhow::Result<()> { case.apply_changes(changes, None); // `import sub.a` should no longer resolve - assert!(resolve_module(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_none()); + assert!( + resolve_module_confident(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_none() + ); assert!(!init_file.exists(case.db())); assert!(!a_file.exists(case.db())); @@ -890,8 +898,12 @@ fn directory_renamed() -> anyhow::Result<()> { let bar = case.system_file(case.project_path("bar.py")).unwrap(); - assert!(resolve_module(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_some()); - assert!(resolve_module(case.db(), &ModuleName::new_static("foo.baz").unwrap()).is_none()); + assert!( + resolve_module_confident(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_some() + ); + assert!( + resolve_module_confident(case.db(), &ModuleName::new_static("foo.baz").unwrap()).is_none() + ); let sub_path = case.project_path("sub"); let sub_init = case @@ -915,9 +927,13 @@ fn directory_renamed() -> anyhow::Result<()> { case.apply_changes(changes, None); // `import sub.a` should no longer resolve - assert!(resolve_module(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_none()); + assert!( + resolve_module_confident(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_none() + ); // `import foo.baz` should now resolve - assert!(resolve_module(case.db(), &ModuleName::new_static("foo.baz").unwrap()).is_some()); + assert!( + resolve_module_confident(case.db(), &ModuleName::new_static("foo.baz").unwrap()).is_some() + ); // The old paths are no longer tracked assert!(!sub_init.exists(case.db())); @@ -950,7 +966,9 @@ fn directory_deleted() -> anyhow::Result<()> { let bar = case.system_file(case.project_path("bar.py")).unwrap(); - assert!(resolve_module(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_some()); + assert!( + resolve_module_confident(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_some() + ); let sub_path = case.project_path("sub"); @@ -970,7 +988,9 @@ fn directory_deleted() -> anyhow::Result<()> { case.apply_changes(changes, None); // `import sub.a` should no longer resolve - assert!(resolve_module(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_none()); + assert!( + resolve_module_confident(case.db(), &ModuleName::new_static("sub.a").unwrap()).is_none() + ); assert!(!init_file.exists(case.db())); assert!(!a_file.exists(case.db())); @@ -999,7 +1019,7 @@ fn search_path() -> anyhow::Result<()> { let site_packages = case.root_path().join("site_packages"); assert_eq!( - resolve_module(case.db(), &ModuleName::new("a").unwrap()), + resolve_module_confident(case.db(), &ModuleName::new("a").unwrap()), None ); @@ -1009,7 +1029,7 @@ fn search_path() -> anyhow::Result<()> { case.apply_changes(changes, None); - assert!(resolve_module(case.db(), &ModuleName::new_static("a").unwrap()).is_some()); + assert!(resolve_module_confident(case.db(), &ModuleName::new_static("a").unwrap()).is_some()); case.assert_indexed_project_files([case.system_file(case.project_path("bar.py")).unwrap()]); Ok(()) @@ -1022,7 +1042,7 @@ fn add_search_path() -> anyhow::Result<()> { let site_packages = case.project_path("site_packages"); std::fs::create_dir_all(site_packages.as_std_path())?; - assert!(resolve_module(case.db(), &ModuleName::new_static("a").unwrap()).is_none()); + assert!(resolve_module_confident(case.db(), &ModuleName::new_static("a").unwrap()).is_none()); // Register site-packages as a search path. case.update_options(Options { @@ -1040,7 +1060,7 @@ fn add_search_path() -> anyhow::Result<()> { case.apply_changes(changes, None); - assert!(resolve_module(case.db(), &ModuleName::new_static("a").unwrap()).is_some()); + assert!(resolve_module_confident(case.db(), &ModuleName::new_static("a").unwrap()).is_some()); Ok(()) } @@ -1172,7 +1192,7 @@ fn changed_versions_file() -> anyhow::Result<()> { // Unset the custom typeshed directory. assert_eq!( - resolve_module(case.db(), &ModuleName::new("os").unwrap()), + resolve_module_confident(case.db(), &ModuleName::new("os").unwrap()), None ); @@ -1187,7 +1207,7 @@ fn changed_versions_file() -> anyhow::Result<()> { case.apply_changes(changes, None); - assert!(resolve_module(case.db(), &ModuleName::new("os").unwrap()).is_some()); + assert!(resolve_module_confident(case.db(), &ModuleName::new("os").unwrap()).is_some()); Ok(()) } @@ -1410,7 +1430,7 @@ mod unix { Ok(()) })?; - let baz = resolve_module(case.db(), &ModuleName::new_static("bar.baz").unwrap()) + let baz = resolve_module_confident(case.db(), &ModuleName::new_static("bar.baz").unwrap()) .expect("Expected bar.baz to exist in site-packages."); let baz_project = case.project_path("bar/baz.py"); let baz_file = baz.file(case.db()).unwrap(); @@ -1486,7 +1506,7 @@ mod unix { Ok(()) })?; - let baz = resolve_module(case.db(), &ModuleName::new_static("bar.baz").unwrap()) + let baz = resolve_module_confident(case.db(), &ModuleName::new_static("bar.baz").unwrap()) .expect("Expected bar.baz to exist in site-packages."); let baz_file = baz.file(case.db()).unwrap(); let bar_baz = case.project_path("bar/baz.py"); @@ -1591,7 +1611,7 @@ mod unix { Ok(()) })?; - let baz = resolve_module(case.db(), &ModuleName::new_static("bar.baz").unwrap()) + let baz = resolve_module_confident(case.db(), &ModuleName::new_static("bar.baz").unwrap()) .expect("Expected bar.baz to exist in site-packages."); let baz_site_packages_path = case.project_path(".venv/lib/python3.12/site-packages/bar/baz.py"); @@ -1854,11 +1874,11 @@ fn rename_files_casing_only() -> anyhow::Result<()> { let mut case = setup([("lib.py", "class Foo: ...")])?; assert!( - resolve_module(case.db(), &ModuleName::new("lib").unwrap()).is_some(), + resolve_module_confident(case.db(), &ModuleName::new("lib").unwrap()).is_some(), "Expected `lib` module to exist." ); assert_eq!( - resolve_module(case.db(), &ModuleName::new("Lib").unwrap()), + resolve_module_confident(case.db(), &ModuleName::new("Lib").unwrap()), None, "Expected `Lib` module not to exist" ); @@ -1891,13 +1911,13 @@ fn rename_files_casing_only() -> anyhow::Result<()> { // Resolving `lib` should now fail but `Lib` should now succeed assert_eq!( - resolve_module(case.db(), &ModuleName::new("lib").unwrap()), + resolve_module_confident(case.db(), &ModuleName::new("lib").unwrap()), None, "Expected `lib` module to no longer exist." ); assert!( - resolve_module(case.db(), &ModuleName::new("Lib").unwrap()).is_some(), + resolve_module_confident(case.db(), &ModuleName::new("Lib").unwrap()).is_some(), "Expected `Lib` module to exist" ); diff --git a/crates/ty_ide/src/all_symbols.rs b/crates/ty_ide/src/all_symbols.rs index 5f5774cd69..c7282a5fc7 100644 --- a/crates/ty_ide/src/all_symbols.rs +++ b/crates/ty_ide/src/all_symbols.rs @@ -20,7 +20,7 @@ pub fn all_symbols<'db>( let typing_extensions = ModuleName::new("typing_extensions").unwrap(); let is_typing_extensions_available = importing_from.is_stub(db) - || resolve_real_shadowable_module(db, &typing_extensions).is_some(); + || resolve_real_shadowable_module(db, importing_from, &typing_extensions).is_some(); let results = std::sync::Mutex::new(Vec::new()); { diff --git a/crates/ty_python_semantic/resources/mdtest/import/workspaces.md b/crates/ty_python_semantic/resources/mdtest/import/workspaces.md index 430860a562..4b6eb75165 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/workspaces.md +++ b/crates/ty_python_semantic/resources/mdtest/import/workspaces.md @@ -6,6 +6,15 @@ python file in some random workspace, and so we need to be more tolerant of situ fly in a published package, cases where we're not configured as well as we'd like, or cases where two projects in a monorepo have conflicting definitions (but we want to analyze both at once). +In practice these tests cover what we call "desperate module resolution" which, when an import +fails, results in us walking up the ancestor directories of the importing file and trying those as +"desperate search-paths". + +Currently desperate search-paths are restricted to subdirectories of the first-party search-path +(the directory you're running `ty` in). Currently we only consider one desperate search-path: the +closest ancestor directory containing a `pyproject.toml`. In the future we may want to try every +ancestor `pyproject.toml` or every ancestor directory. + ## Invalid Names While you can't syntactically refer to a module with an invalid name (i.e. one with a `-`, or that @@ -18,9 +27,10 @@ strings and does in fact allow syntactically invalid module names. ### Current File Is Invalid Module Name -Relative and absolute imports should resolve fine in a file that isn't a valid module name. +Relative and absolute imports should resolve fine in a file that isn't a valid module name (in this +case, it could be imported via `importlib.import_module`). -`my-main.py`: +`tests/my-mod.py`: ```py # TODO: there should be no errors in this file @@ -37,13 +47,13 @@ reveal_type(mod2.y) # revealed: Unknown reveal_type(mod3.z) # revealed: int ``` -`mod1.py`: +`tests/mod1.py`: ```py x: int = 1 ``` -`mod2.py`: +`tests/mod2.py`: ```py y: int = 2 @@ -57,13 +67,16 @@ z: int = 2 ### Current Directory Is Invalid Module Name -Relative and absolute imports should resolve fine in a dir that isn't a valid module name. +If python files are rooted in a directory with an invalid module name and they relatively import +each other, there's probably no coherent explanation for what's going on and it's fine that the +relative import don't resolve (but maybe we could provide some good diagnostics). -`my-tests/main.py`: +This is a case that sufficient desperation might "accidentally" make work, so it's included here as +a canary in the coal mine. + +`my-tests/mymod.py`: ```py -# TODO: there should be no errors in this file - # error: [unresolved-import] from .mod1 import x @@ -94,46 +107,97 @@ y: int = 2 z: int = 2 ``` -### Current Directory Is Invalid Package Name +### Ancestor Directory Is Invalid Module Name -Relative and absolute imports should resolve fine in a dir that isn't a valid package name, even if -it contains an `__init__.py`: +Relative and absolute imports *could* resolve fine in the first-party search-path, even if one of +the ancestor dirs is an invalid module. i.e. in this case we will be inclined to compute module +names like `my-proj.tests.mymod`, but it could be that in practice the user always runs this code +rooted in the `my-proj` directory. -`my-tests/__init__.py`: +This case is hard for us to detect and handle in a principled way, but two more extreme kinds of +desperation could handle this: + +- try every ancestor as a desperate search-path +- try the closest ancestor with an invalid module name as a desperate search-path + +The second one is a bit messed up because it could result in situations where someone can get a +worse experience because a directory happened to *not* be invalid as a module name (`myproj` or +`my_proj`). + +`my-proj/tests/mymod.py`: ```py -``` - -`my-tests/main.py`: - -```py -# TODO: there should be no errors in this file +# TODO: it would be *nice* if there were no errors in this file # error: [unresolved-import] from .mod1 import x # error: [unresolved-import] from . import mod2 + +# error: [unresolved-import] import mod3 reveal_type(x) # revealed: Unknown reveal_type(mod2.y) # revealed: Unknown -reveal_type(mod3.z) # revealed: int +reveal_type(mod3.z) # revealed: Unknown ``` -`my-tests/mod1.py`: +`my-proj/tests/mod1.py`: ```py x: int = 1 ``` -`my-tests/mod2.py`: +`my-proj/tests/mod2.py`: ```py y: int = 2 ``` -`mod3.py`: +`my-proj/mod3.py`: + +```py +z: int = 2 +``` + +### Ancestor Directory Above `pyproject.toml` is invalid + +Like the previous tests but with a `pyproject.toml` existing between the invalid name and the python +files. This is an "easier" case in case we use the `pyproject.toml` as a hint about what's going on. + +`my-proj/pyproject.toml`: + +```text +name = "my_proj" +version = "0.1.0" +``` + +`my-proj/tests/main.py`: + +```py +from .mod1 import x +from . import mod2 +import mod3 + +reveal_type(x) # revealed: int +reveal_type(mod2.y) # revealed: int +reveal_type(mod3.z) # revealed: int +``` + +`my-proj/tests/mod1.py`: + +```py +x: int = 1 +``` + +`my-proj/tests/mod2.py`: + +```py +y: int = 2 +``` + +`my-proj/mod3.py`: ```py z: int = 2 @@ -141,7 +205,7 @@ z: int = 2 ## Multiple Projects -It's common for a monorepo to define many separate projects that may or may not depend on eachother +It's common for a monorepo to define many separate projects that may or may not depend on each other and are stitched together with a package manager like `uv` or `poetry`, often as editables. In this case, especially when running as an LSP, we want to be able to analyze all of the projects at once, allowing us to reuse results between projects, without getting confused about things that only make @@ -150,7 +214,7 @@ sense when analyzing the project separately. The following tests will feature two projects, `a` and `b` where the "real" packages are found under `src/` subdirectories (and we've been configured to understand that), but each project also contains other python files in their roots or subdirectories that contains python files which relatively -import eachother and also absolutely import the main package of the project. All of these imports +import each other and also absolutely import the main package of the project. All of these imports *should* resolve. Often the fact that there is both an `a` and `b` project seemingly won't matter, but many possible @@ -164,13 +228,36 @@ following examples include them in case they help. Here we have fairly typical situation where there are two projects `aproj` and `bproj` where the "real" packages are found under `src/` subdirectories, but each project also contains a `tests/` -directory that contains python files which relatively import eachother and also absolutely import +directory that contains python files which relatively import each other and also absolutely import the package they test. All of these imports *should* resolve. ```toml [environment] -# This is similar to what we would compute for installed editables -extra-paths = ["aproj/src/", "bproj/src/"] +# Setup a venv with editables for aproj/src/ and bproj/src/ +python = "/.venv" +``` + +`/.venv/pyvenv.cfg`: + +```cfg +home = /doo/doo/wop/cpython-3.13.2-macos-aarch64-none/bin +``` + +`/doo/doo/wop/cpython-3.13.2-macos-aarch64-none/bin/python`: + +```text +``` + +`/.venv//a.pth`: + +```pth +aproj/src/ +``` + +`/.venv//b.pth`: + +```pth +bproj/src/ ``` `aproj/tests/test1.py`: @@ -239,16 +326,60 @@ version = "0.1.0" y: str = "20" ``` -### Tests Directory With Ambiguous Project Directories +### Tests Directory With Ambiguous Project Directories Via Editables The same situation as the previous test but instead of the project `a` being in a directory `aproj` to disambiguate, we now need to avoid getting confused about whether `a/` or `a/src/a/` is the package `a` while still resolving imports. +Unfortunately this is a quite difficult square to circle as `a/` is a namespace package of `a` and +`a/src/a/` is a regular package of `a`. **This is a very bad situation you're not supposed to ever +create, and we are now very sensitive to precise search-path ordering.** + +Here the use of editables means that `a/` has higher priority than `a/src/a/`. + +Somehow this results in `a/tests/test1.py` being able to resolve `.setup` but not `.`. + +My best guess is that in this state we can resolve regular modules in `a/tests/` but not namespace +packages because we have some extra validation for namespace packages conflicted by regular +packages, but that validation isn't applied when we successfully resolve a submodule of the +namespace package. + +In this case, as we find that `a/tests/test1.py` matches on the first-party path as `a.tests.test1` +and is syntactically valid. We then resolve `a.tests.test1` and because the namespace package +(`/a/`) comes first we succeed. We then syntactically compute `.` to be `a.tests`. + +When we go to lookup `a.tests.setup`, whatever grace that allowed `a.tests.test1` to resolve still +works so it resolves too. However when we try to resolve `a.tests` on its own some additional +validation rejects the namespace package conflicting with the regular package. + ```toml [environment] -# This is similar to what we would compute for installed editables -extra-paths = ["a/src/", "b/src/"] +# Setup a venv with editables for a/src/ and b/src/ +python = "/.venv" +``` + +`/.venv/pyvenv.cfg`: + +```cfg +home = /doo/doo/wop/cpython-3.13.2-macos-aarch64-none/bin +``` + +`/doo/doo/wop/cpython-3.13.2-macos-aarch64-none/bin/python`: + +```text +``` + +`/.venv//a.pth`: + +```pth +a/src/ +``` + +`/.venv//b.pth`: + +```pth +b/src/ ``` `a/tests/test1.py`: @@ -256,7 +387,6 @@ extra-paths = ["a/src/", "b/src/"] ```py # TODO: there should be no errors in this file. -# error: [unresolved-import] from .setup import x # error: [unresolved-import] @@ -264,7 +394,7 @@ from . import setup from a import y import a -reveal_type(x) # revealed: Unknown +reveal_type(x) # revealed: int reveal_type(setup.x) # revealed: Unknown reveal_type(y) # revealed: int reveal_type(a.y) # revealed: int @@ -294,7 +424,6 @@ y: int = 10 ```py # TODO: there should be no errors in this file -# error: [unresolved-import] from .setup import x # error: [unresolved-import] @@ -302,7 +431,7 @@ from . import setup from b import y import b -reveal_type(x) # revealed: Unknown +reveal_type(x) # revealed: str reveal_type(setup.x) # revealed: Unknown reveal_type(y) # revealed: str reveal_type(b.y) # revealed: str @@ -327,10 +456,15 @@ version = "0.1.0" y: str = "20" ``` -### Tests Package With Ambiguous Project Directories +### Tests Directory With Ambiguous Project Directories Via `extra-paths` -The same situation as the previous test but `tests/__init__.py` is also defined, in case that -complicates the situation. +The same situation as the previous test but instead of using editables we use `extra-paths` which +have higher priority than the first-party search-path. Thus, `/a/src/a/` is always seen before +`/a/`. + +In this case everything works well because the namespace package `a.tests` (`a/tests/`) is +completely hidden by the regular package `a` (`a/src/a/`) and so we immediately enter desperate +resolution and use the now-unambiguous namespace package `tests`. ```toml [environment] @@ -340,27 +474,17 @@ extra-paths = ["a/src/", "b/src/"] `a/tests/test1.py`: ```py -# TODO: there should be no errors in this file. - -# error: [unresolved-import] from .setup import x - -# error: [unresolved-import] from . import setup from a import y import a -reveal_type(x) # revealed: Unknown -reveal_type(setup.x) # revealed: Unknown +reveal_type(x) # revealed: int +reveal_type(setup.x) # revealed: int reveal_type(y) # revealed: int reveal_type(a.y) # revealed: int ``` -`a/tests/__init__.py`: - -```py -``` - `a/tests/setup.py`: ```py @@ -383,27 +507,17 @@ y: int = 10 `b/tests/test1.py`: ```py -# TODO: there should be no errors in this file - -# error: [unresolved-import] from .setup import x - -# error: [unresolved-import] from . import setup from b import y import b -reveal_type(x) # revealed: Unknown -reveal_type(setup.x) # revealed: Unknown +reveal_type(x) # revealed: str +reveal_type(setup.x) # revealed: str reveal_type(y) # revealed: str reveal_type(b.y) # revealed: str ``` -`b/tests/__init__.py`: - -```py -``` - `b/tests/setup.py`: ```py @@ -431,21 +545,16 @@ that `import main` and expect that to work. `a/tests/test1.py`: ```py -# TODO: there should be no errors in this file. - from .setup import x from . import setup -# error: [unresolved-import] from main import y - -# error: [unresolved-import] import main reveal_type(x) # revealed: int reveal_type(setup.x) # revealed: int -reveal_type(y) # revealed: Unknown -reveal_type(main.y) # revealed: Unknown +reveal_type(y) # revealed: int +reveal_type(main.y) # revealed: int ``` `a/tests/setup.py`: @@ -470,113 +579,16 @@ y: int = 10 `b/tests/test1.py`: ```py -# TODO: there should be no errors in this file - from .setup import x from . import setup -# error: [unresolved-import] from main import y - -# error: [unresolved-import] import main reveal_type(x) # revealed: str reveal_type(setup.x) # revealed: str -reveal_type(y) # revealed: Unknown -reveal_type(main.y) # revealed: Unknown -``` - -`b/tests/setup.py`: - -```py -x: str = "2" -``` - -`b/pyproject.toml`: - -```text -name = "a" -version = "0.1.0" -``` - -`b/main.py`: - -```py -y: str = "20" -``` - -### Tests Package Absolute Importing `main.py` - -The same as the previous case but `tests/__init__.py` exists in case that causes different issues. - -`a/tests/test1.py`: - -```py -# TODO: there should be no errors in this file. - -from .setup import x -from . import setup - -# error: [unresolved-import] -from main import y - -# error: [unresolved-import] -import main - -reveal_type(x) # revealed: int -reveal_type(setup.x) # revealed: int -reveal_type(y) # revealed: Unknown -reveal_type(main.y) # revealed: Unknown -``` - -`a/tests/__init__.py`: - -```py -``` - -`a/tests/setup.py`: - -```py -x: int = 1 -``` - -`a/pyproject.toml`: - -```text -name = "a" -version = "0.1.0" -``` - -`a/main.py`: - -```py -y: int = 10 -``` - -`b/tests/test1.py`: - -```py -# TODO: there should be no errors in this file - -from .setup import x -from . import setup - -# error: [unresolved-import] -from main import y - -# error: [unresolved-import] -import main - -reveal_type(x) # revealed: str -reveal_type(setup.x) # revealed: str -reveal_type(y) # revealed: Unknown -reveal_type(main.y) # revealed: Unknown -``` - -`b/tests/__init__.py`: - -```py +reveal_type(y) # revealed: str +reveal_type(main.y) # revealed: str ``` `b/tests/setup.py`: @@ -606,16 +618,11 @@ imports it. `a/main.py`: ```py -# TODO: there should be no errors in this file. - -# error: [unresolved-import] from utils import x - -# error: [unresolved-import] import utils -reveal_type(x) # revealed: Unknown -reveal_type(utils.x) # revealed: Unknown +reveal_type(x) # revealed: int +reveal_type(utils.x) # revealed: int ``` `a/utils/__init__.py`: @@ -634,16 +641,11 @@ version = "0.1.0" `b/main.py`: ```py -# TODO: there should be no errors in this file. - -# error: [unresolved-import] from utils import x - -# error: [unresolved-import] import utils -reveal_type(x) # revealed: Unknown -reveal_type(utils.x) # revealed: Unknown +reveal_type(x) # revealed: str +reveal_type(utils.x) # revealed: str ``` `b/utils/__init__.py`: diff --git a/crates/ty_python_semantic/src/dunder_all.rs b/crates/ty_python_semantic/src/dunder_all.rs index 17f1706b7e..1803037d6d 100644 --- a/crates/ty_python_semantic/src/dunder_all.rs +++ b/crates/ty_python_semantic/src/dunder_all.rs @@ -166,7 +166,7 @@ impl<'db> DunderAllNamesCollector<'db> { ) -> Option<&'db FxHashSet> { let module_name = ModuleName::from_import_statement(self.db, self.file, import_from).ok()?; - let module = resolve_module(self.db, &module_name)?; + let module = resolve_module(self.db, self.file, &module_name)?; dunder_all_names(self.db, module.file(self.db)?) } diff --git a/crates/ty_python_semantic/src/lib.rs b/crates/ty_python_semantic/src/lib.rs index 8c1776e154..cf386839c9 100644 --- a/crates/ty_python_semantic/src/lib.rs +++ b/crates/ty_python_semantic/src/lib.rs @@ -13,8 +13,8 @@ pub use diagnostic::add_inferred_python_version_hint_to_diagnostic; pub use module_name::{ModuleName, ModuleNameResolutionError}; pub use module_resolver::{ KnownModule, Module, SearchPath, SearchPathValidationError, SearchPaths, all_modules, - list_modules, resolve_module, resolve_real_module, resolve_real_shadowable_module, - system_module_search_paths, + list_modules, resolve_module, resolve_module_confident, resolve_real_module, + resolve_real_module_confident, resolve_real_shadowable_module, system_module_search_paths, }; pub use program::{ Program, ProgramSettings, PythonVersionFileSource, PythonVersionSource, diff --git a/crates/ty_python_semantic/src/module_resolver/mod.rs b/crates/ty_python_semantic/src/module_resolver/mod.rs index cc541d9b31..08f6ee3fc4 100644 --- a/crates/ty_python_semantic/src/module_resolver/mod.rs +++ b/crates/ty_python_semantic/src/module_resolver/mod.rs @@ -6,7 +6,10 @@ pub use module::Module; pub use path::{SearchPath, SearchPathValidationError}; pub use resolver::SearchPaths; pub(crate) use resolver::file_to_module; -pub use resolver::{resolve_module, resolve_real_module, resolve_real_shadowable_module}; +pub use resolver::{ + resolve_module, resolve_module_confident, resolve_real_module, resolve_real_module_confident, + resolve_real_shadowable_module, +}; use ruff_db::system::SystemPath; use crate::Db; diff --git a/crates/ty_python_semantic/src/module_resolver/path.rs b/crates/ty_python_semantic/src/module_resolver/path.rs index cb524cb4ac..5200396dc1 100644 --- a/crates/ty_python_semantic/src/module_resolver/path.rs +++ b/crates/ty_python_semantic/src/module_resolver/path.rs @@ -608,6 +608,18 @@ impl SearchPath { #[must_use] pub(crate) fn relativize_system_path(&self, path: &SystemPath) -> Option { + self.relativize_system_path_only(path) + .map(|relative_path| ModulePath { + search_path: self.clone(), + relative_path: relative_path.as_utf8_path().to_path_buf(), + }) + } + + #[must_use] + pub(crate) fn relativize_system_path_only<'a>( + &self, + path: &'a SystemPath, + ) -> Option<&'a SystemPath> { if path .extension() .is_some_and(|extension| !self.is_valid_extension(extension)) @@ -621,14 +633,7 @@ impl SearchPath { | SearchPathInner::StandardLibraryCustom(search_path) | SearchPathInner::StandardLibraryReal(search_path) | SearchPathInner::SitePackages(search_path) - | SearchPathInner::Editable(search_path) => { - path.strip_prefix(search_path) - .ok() - .map(|relative_path| ModulePath { - search_path: self.clone(), - relative_path: relative_path.as_utf8_path().to_path_buf(), - }) - } + | SearchPathInner::Editable(search_path) => path.strip_prefix(search_path).ok(), SearchPathInner::StandardLibraryVendored(_) => None, } } @@ -783,7 +788,7 @@ impl fmt::Display for SearchPath { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub(super) enum SystemOrVendoredPathRef<'db> { System(&'db SystemPath), Vendored(&'db VendoredPath), diff --git a/crates/ty_python_semantic/src/module_resolver/resolver.rs b/crates/ty_python_semantic/src/module_resolver/resolver.rs index ecf92d2d83..123b4ac31e 100644 --- a/crates/ty_python_semantic/src/module_resolver/resolver.rs +++ b/crates/ty_python_semantic/src/module_resolver/resolver.rs @@ -1,8 +1,31 @@ /*! -This module principally provides two routines for resolving a particular module -name to a `Module`: [`resolve_module`] and [`resolve_real_module`]. You'll -usually want the former, unless you're certain you want to forbid stubs, in -which case, use the latter. +This module principally provides several routines for resolving a particular module +name to a `Module`: + +* [`file_to_module`][]: resolves the module `.` (often as the first step in resolving `.`) +* [`resolve_module`][]: resolves an absolute module name + +You may notice that we actually provide `resolve_(real)_(shadowable)_module_(confident)`. +You almost certainly just want [`resolve_module`][]. The other variations represent +restrictions to answer specific kinds of questions, usually to empower IDE features. + +* The `real` variation disallows all stub files, including the vendored typeshed. + This enables the goto-definition ("real") vs goto-declaration ("stub or real") distinction. + +* The `confident` variation disallows "desperate resolution", which is a fallback + mode where we start trying to use ancestor directories of the importing file + as search-paths, but only if we failed to resolve it with the normal search-paths. + This is mostly just a convenience for cases where we don't want to try to define + the importing file (resolving a `KnownModule` and tests). + +* The `shadowable` variation disables some guards that prevents third-party code + from shadowing any vendored non-stdlib `KnownModule`. In particular `typing_extensions`, + which we vendor and heavily assume the contents of (and so don't ever want to shadow). + This enables checking if the user *actually* has `typing_extensions` installed, + in which case it's ok to suggest it in features like auto-imports. + +There is some awkwardness to the structure of the code to specifically enable caching +of queries, as module resolution happens a lot and involves a lot of disk access. For implementors, see `import-resolution-diagram.svg` for a flow diagram that specifies ty's implementation of Python's import resolution algorithm. @@ -33,14 +56,51 @@ use super::module::{Module, ModuleKind}; use super::path::{ModulePath, SearchPath, SearchPathValidationError, SystemOrVendoredPathRef}; /// Resolves a module name to a module. -pub fn resolve_module<'db>(db: &'db dyn Db, module_name: &ModuleName) -> Option> { +pub fn resolve_module<'db>( + db: &'db dyn Db, + importing_file: File, + module_name: &ModuleName, +) -> Option> { + let interned_name = ModuleNameIngredient::new(db, module_name, ModuleResolveMode::StubsAllowed); + + resolve_module_query(db, interned_name) + .or_else(|| desperately_resolve_module(db, importing_file, interned_name)) +} + +/// Resolves a module name to a module, without desperate resolution available. +/// +/// This is appropriate for resolving a `KnownModule`, or cases where for whatever reason +/// we don't have a well-defined importing file. +pub fn resolve_module_confident<'db>( + db: &'db dyn Db, + module_name: &ModuleName, +) -> Option> { let interned_name = ModuleNameIngredient::new(db, module_name, ModuleResolveMode::StubsAllowed); resolve_module_query(db, interned_name) } /// Resolves a module name to a module (stubs not allowed). -pub fn resolve_real_module<'db>(db: &'db dyn Db, module_name: &ModuleName) -> Option> { +pub fn resolve_real_module<'db>( + db: &'db dyn Db, + importing_file: File, + module_name: &ModuleName, +) -> Option> { + let interned_name = + ModuleNameIngredient::new(db, module_name, ModuleResolveMode::StubsNotAllowed); + + resolve_module_query(db, interned_name) + .or_else(|| desperately_resolve_module(db, importing_file, interned_name)) +} + +/// Resolves a module name to a module, without desperate resolution available (stubs not allowed). +/// +/// This is appropriate for resolving a `KnownModule`, or cases where for whatever reason +/// we don't have a well-defined importing file. +pub fn resolve_real_module_confident<'db>( + db: &'db dyn Db, + module_name: &ModuleName, +) -> Option> { let interned_name = ModuleNameIngredient::new(db, module_name, ModuleResolveMode::StubsNotAllowed); @@ -60,6 +120,7 @@ pub fn resolve_real_module<'db>(db: &'db dyn Db, module_name: &ModuleName) -> Op /// are involved in an import cycle with `builtins`. pub fn resolve_real_shadowable_module<'db>( db: &'db dyn Db, + importing_file: File, module_name: &ModuleName, ) -> Option> { let interned_name = ModuleNameIngredient::new( @@ -69,6 +130,7 @@ pub fn resolve_real_shadowable_module<'db>( ); resolve_module_query(db, interned_name) + .or_else(|| desperately_resolve_module(db, importing_file, interned_name)) } /// Which files should be visible when doing a module query @@ -181,6 +243,55 @@ fn resolve_module_query<'db>( Some(module) } +/// Like `resolve_module_query` but for cases where it failed to resolve the module +/// and we are now Getting Desperate and willing to try the ancestor directories of +/// the `importing_file` as potential temporary search paths that are private +/// to this import. +/// +/// The reason this is split out is because in 99.9% of cases `resolve_module_query` +/// will find the right answer (or no valid answer exists), and we want it to be +/// aggressively cached. Including the `importing_file` as part of that query would +/// trash the caching of import resolution between files. +/// +/// TODO: should (some) of this also be cached? If an entire directory of python files +/// is misunderstood we'll end up in here a lot. +fn desperately_resolve_module<'db>( + db: &'db dyn Db, + importing_file: File, + module_name: ModuleNameIngredient<'db>, +) -> Option> { + let name = module_name.name(db); + let mode = module_name.mode(db); + let _span = tracing::trace_span!("desperately_resolve_module", %name).entered(); + + let Some(resolved) = desperately_resolve_name(db, importing_file, name, mode) else { + tracing::debug!("Module `{name}` not found while looking in parent dirs"); + return None; + }; + + let module = match resolved { + ResolvedName::FileModule(module) => { + tracing::trace!( + "Resolved module `{name}` to `{path}`", + path = module.file.path(db) + ); + Module::file_module( + db, + name.clone(), + module.kind, + module.search_path, + module.file, + ) + } + ResolvedName::NamespacePackage => { + tracing::trace!("Module `{name}` is a namespace package"); + Module::namespace_package(db, name.clone()) + } + }; + + Some(module) +} + /// Resolves the module for the given path. /// /// Returns `None` if the path is not a module locatable via any of the known search paths. @@ -201,13 +312,33 @@ pub(crate) fn path_to_module<'db>(db: &'db dyn Db, path: &FilePath) -> Option` in the file itself, +/// and indeed, one of its primary jobs is resolving `.` to derive the module name of `.`. +/// This intuition is particularly useful for understanding why it's correct that we pass +/// the file itself as `importing_file` to various subroutines. #[salsa::tracked(heap_size=ruff_memory_usage::heap_size)] pub(crate) fn file_to_module(db: &dyn Db, file: File) -> Option> { let _span = tracing::trace_span!("file_to_module", ?file).entered(); let path = SystemOrVendoredPathRef::try_from_file(db, file)?; - let module_name = search_paths(db, ModuleResolveMode::StubsAllowed).find_map(|candidate| { + file_to_module_impl( + db, + file, + path, + search_paths(db, ModuleResolveMode::StubsAllowed), + ) + .or_else(|| file_to_module_impl(db, file, path, desperate_search_paths(db, file).iter())) +} + +fn file_to_module_impl<'db, 'a>( + db: &'db dyn Db, + file: File, + path: SystemOrVendoredPathRef<'a>, + mut search_paths: impl Iterator, +) -> Option> { + let module_name = search_paths.find_map(|candidate: &SearchPath| { let relative_path = match path { SystemOrVendoredPathRef::System(path) => candidate.relativize_system_path(path), SystemOrVendoredPathRef::Vendored(path) => candidate.relativize_vendored_path(path), @@ -219,7 +350,7 @@ pub(crate) fn file_to_module(db: &dyn Db, file: File) -> Option> { // If it doesn't, then that means that multiple modules have the same name in different // root paths, but that the module corresponding to `path` is in a lower priority search path, // in which case we ignore it. - let module = resolve_module(db, &module_name)?; + let module = resolve_module(db, file, &module_name)?; let module_file = module.file(db)?; if file.path(db) == module_file.path(db) { @@ -230,7 +361,7 @@ pub(crate) fn file_to_module(db: &dyn Db, file: File) -> Option> { // If a .py and .pyi are both defined, the .pyi will be the one returned by `resolve_module().file`, // which would make us erroneously believe the `.py` is *not* also this module (breaking things // like relative imports). So here we try `resolve_real_module().file` to cover both cases. - let module = resolve_real_module(db, &module_name)?; + let module = resolve_real_module(db, file, &module_name)?; let module_file = module.file(db)?; if file.path(db) == module_file.path(db) { return Some(module); @@ -250,6 +381,58 @@ pub(crate) fn search_paths(db: &dyn Db, resolve_mode: ModuleResolveMode) -> Sear Program::get(db).search_paths(db).iter(db, resolve_mode) } +/// Get the search-paths that should be used for desperate resolution of imports in this file +/// +/// Currently this is "the closest ancestor dir that contains a pyproject.toml", which is +/// a completely arbitrary decision. We could potentially change this to return an iterator +/// of every ancestor with a pyproject.toml or every ancestor. +/// +/// For now this works well in common cases where we have some larger workspace that contains +/// one or more python projects in sub-directories, and those python projects assume that +/// absolute imports resolve relative to the pyproject.toml they live under. +/// +/// Being so strict minimizes concerns about this going off a lot and doing random +/// chaotic things. In particular, all files under a given pyproject.toml will currently +/// agree on this being their desperate search-path, which is really nice. +#[salsa::tracked(heap_size=ruff_memory_usage::heap_size)] +fn desperate_search_paths(db: &dyn Db, importing_file: File) -> Option { + let system = db.system(); + let importing_path = importing_file.path(db).as_system_path()?; + + // Only allow this if the importing_file is under the first-party search path + let (base_path, rel_path) = + search_paths(db, ModuleResolveMode::StubsAllowed).find_map(|search_path| { + if !search_path.is_first_party() { + return None; + } + Some(( + search_path.as_system_path()?, + search_path.relativize_system_path_only(importing_path)?, + )) + })?; + + // Read the revision on the corresponding file root to + // register an explicit dependency on this directory. When + // the revision gets bumped, the cache that Salsa creates + // for this routine will be invalidated. + // + // (This is conditional because ruff uses this code too and doesn't set roots) + if let Some(root) = db.files().root(db, base_path) { + let _ = root.revision(db); + } + + // Only allow searching up to the first-party path's root + for rel_dir in rel_path.ancestors() { + let candidate_path = base_path.join(rel_dir); + if system.path_exists(&candidate_path.join("pyproject.toml")) + || system.path_exists(&candidate_path.join("ty.toml")) + { + let search_path = SearchPath::first_party(system, candidate_path).ok()?; + return Some(search_path); + } + } + None +} #[derive(Clone, Debug, PartialEq, Eq, get_size2::GetSize)] pub struct SearchPaths { /// Search paths that have been statically determined purely from reading @@ -756,6 +939,30 @@ struct ModuleNameIngredient<'db> { /// Given a module name and a list of search paths in which to lookup modules, /// attempt to resolve the module name fn resolve_name(db: &dyn Db, name: &ModuleName, mode: ModuleResolveMode) -> Option { + let search_paths = search_paths(db, mode); + resolve_name_impl(db, name, mode, search_paths) +} + +/// Like `resolve_name` but for cases where it failed to resolve the module +/// and we are now Getting Desperate and willing to try the ancestor directories of +/// the `importing_file` as potential temporary search paths that are private +/// to this import. +fn desperately_resolve_name( + db: &dyn Db, + importing_file: File, + name: &ModuleName, + mode: ModuleResolveMode, +) -> Option { + let search_paths = desperate_search_paths(db, importing_file); + resolve_name_impl(db, name, mode, search_paths.iter()) +} + +fn resolve_name_impl<'a>( + db: &dyn Db, + name: &ModuleName, + mode: ModuleResolveMode, + search_paths: impl Iterator, +) -> Option { let program = Program::get(db); let python_version = program.python_version(db); let resolver_state = ResolverContext::new(db, python_version, mode); @@ -765,7 +972,7 @@ fn resolve_name(db: &dyn Db, name: &ModuleName, mode: ModuleResolveMode) -> Opti let stub_name = name.to_stub_package(); let mut is_namespace_package = false; - for search_path in search_paths(db, mode) { + for search_path in search_paths { // When a builtin module is imported, standard module resolution is bypassed: // the module name always resolves to the stdlib module, // even if there's a module of the same name in the first-party root @@ -1409,11 +1616,11 @@ mod tests { .build(); let foo_module_name = ModuleName::new_static("foo").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); assert_eq!( Some(&foo_module), - resolve_module(&db, &foo_module_name).as_ref() + resolve_module_confident(&db, &foo_module_name).as_ref() ); assert_eq!("foo", foo_module.name(&db)); @@ -1435,11 +1642,11 @@ mod tests { .build(); let foo_module_name = ModuleName::new_static("foo").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); assert_eq!( Some(&foo_module), - resolve_module(&db, &foo_module_name).as_ref() + resolve_module_confident(&db, &foo_module_name).as_ref() ); assert_eq!("foo", foo_module.name(&db)); @@ -1467,11 +1674,11 @@ mod tests { .build(); let foo_module_name = ModuleName::new_static("foo").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); assert_eq!( Some(&foo_module), - resolve_module(&db, &foo_module_name).as_ref() + resolve_module_confident(&db, &foo_module_name).as_ref() ); assert_eq!("foo", foo_module.name(&db)); @@ -1494,7 +1701,8 @@ mod tests { .build(); let builtins_module_name = ModuleName::new_static("builtins").unwrap(); - let builtins = resolve_module(&db, &builtins_module_name).expect("builtins to resolve"); + let builtins = + resolve_module_confident(&db, &builtins_module_name).expect("builtins to resolve"); assert_eq!( builtins.file(&db).unwrap().path(&db), @@ -1518,7 +1726,8 @@ mod tests { .build(); let builtins_module_name = ModuleName::new_static("builtins").unwrap(); - let builtins = resolve_module(&db, &builtins_module_name).expect("builtins to resolve"); + let builtins = + resolve_module_confident(&db, &builtins_module_name).expect("builtins to resolve"); assert_eq!( builtins.file(&db).unwrap().path(&db), @@ -1539,11 +1748,11 @@ mod tests { .build(); let functools_module_name = ModuleName::new_static("functools").unwrap(); - let functools_module = resolve_module(&db, &functools_module_name).unwrap(); + let functools_module = resolve_module_confident(&db, &functools_module_name).unwrap(); assert_eq!( Some(&functools_module), - resolve_module(&db, &functools_module_name).as_ref() + resolve_module_confident(&db, &functools_module_name).as_ref() ); assert_eq!(&stdlib, functools_module.search_path(&db).unwrap()); @@ -1596,9 +1805,10 @@ mod tests { let existing_modules = create_module_names(&["asyncio", "functools", "xml.etree"]); for module_name in existing_modules { - let resolved_module = resolve_module(&db, &module_name).unwrap_or_else(|| { - panic!("Expected module {module_name} to exist in the mock stdlib") - }); + let resolved_module = + resolve_module_confident(&db, &module_name).unwrap_or_else(|| { + panic!("Expected module {module_name} to exist in the mock stdlib") + }); let search_path = resolved_module.search_path(&db).unwrap(); assert_eq!( &stdlib, search_path, @@ -1649,7 +1859,7 @@ mod tests { for module_name in nonexisting_modules { assert!( - resolve_module(&db, &module_name).is_none(), + resolve_module_confident(&db, &module_name).is_none(), "Unexpectedly resolved a module for {module_name}" ); } @@ -1692,9 +1902,10 @@ mod tests { ]); for module_name in existing_modules { - let resolved_module = resolve_module(&db, &module_name).unwrap_or_else(|| { - panic!("Expected module {module_name} to exist in the mock stdlib") - }); + let resolved_module = + resolve_module_confident(&db, &module_name).unwrap_or_else(|| { + panic!("Expected module {module_name} to exist in the mock stdlib") + }); let search_path = resolved_module.search_path(&db).unwrap(); assert_eq!( &stdlib, search_path, @@ -1728,7 +1939,7 @@ mod tests { let nonexisting_modules = create_module_names(&["importlib", "xml", "xml.etree"]); for module_name in nonexisting_modules { assert!( - resolve_module(&db, &module_name).is_none(), + resolve_module_confident(&db, &module_name).is_none(), "Unexpectedly resolved a module for {module_name}" ); } @@ -1750,11 +1961,11 @@ mod tests { .build(); let functools_module_name = ModuleName::new_static("functools").unwrap(); - let functools_module = resolve_module(&db, &functools_module_name).unwrap(); + let functools_module = resolve_module_confident(&db, &functools_module_name).unwrap(); assert_eq!( Some(&functools_module), - resolve_module(&db, &functools_module_name).as_ref() + resolve_module_confident(&db, &functools_module_name).as_ref() ); assert_eq!(&src, functools_module.search_path(&db).unwrap()); assert_eq!(ModuleKind::Module, functools_module.kind(&db)); @@ -1777,7 +1988,7 @@ mod tests { .build(); let pydoc_data_topics_name = ModuleName::new_static("pydoc_data.topics").unwrap(); - let pydoc_data_topics = resolve_module(&db, &pydoc_data_topics_name).unwrap(); + let pydoc_data_topics = resolve_module_confident(&db, &pydoc_data_topics_name).unwrap(); assert_eq!("pydoc_data.topics", pydoc_data_topics.name(&db)); assert_eq!(pydoc_data_topics.search_path(&db).unwrap(), &stdlib); @@ -1794,7 +2005,8 @@ mod tests { .build(); let foo_path = src.join("foo/__init__.py"); - let foo_module = resolve_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); + let foo_module = + resolve_module_confident(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); assert_eq!("foo", foo_module.name(&db)); assert_eq!(&src, foo_module.search_path(&db).unwrap()); @@ -1821,7 +2033,8 @@ mod tests { let TestCase { db, src, .. } = TestCaseBuilder::new().with_src_files(SRC).build(); - let foo_module = resolve_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); + let foo_module = + resolve_module_confident(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); let foo_init_path = src.join("foo/__init__.py"); assert_eq!(&src, foo_module.search_path(&db).unwrap()); @@ -1844,8 +2057,9 @@ mod tests { let TestCase { db, src, .. } = TestCaseBuilder::new().with_src_files(SRC).build(); - let foo = resolve_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); - let foo_real = resolve_real_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); + let foo = resolve_module_confident(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); + let foo_real = + resolve_real_module_confident(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); let foo_stub = src.join("foo.pyi"); assert_eq!(&src, foo.search_path(&db).unwrap()); @@ -1870,7 +2084,7 @@ mod tests { let TestCase { db, src, .. } = TestCaseBuilder::new().with_src_files(SRC).build(); let baz_module = - resolve_module(&db, &ModuleName::new_static("foo.bar.baz").unwrap()).unwrap(); + resolve_module_confident(&db, &ModuleName::new_static("foo.bar.baz").unwrap()).unwrap(); let baz_path = src.join("foo/bar/baz.py"); assert_eq!(&src, baz_module.search_path(&db).unwrap()); @@ -1894,7 +2108,8 @@ mod tests { .with_site_packages_files(&[("foo.py", "")]) .build(); - let foo_module = resolve_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); + let foo_module = + resolve_module_confident(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); let foo_src_path = src.join("foo.py"); assert_eq!(&src, foo_module.search_path(&db).unwrap()); @@ -1965,8 +2180,10 @@ mod tests { }, ); - let foo_module = resolve_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); - let bar_module = resolve_module(&db, &ModuleName::new_static("bar").unwrap()).unwrap(); + let foo_module = + resolve_module_confident(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); + let bar_module = + resolve_module_confident(&db, &ModuleName::new_static("bar").unwrap()).unwrap(); assert_ne!(foo_module, bar_module); @@ -2001,7 +2218,7 @@ mod tests { .build(); let foo_module_name = ModuleName::new_static("foo").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); let foo_pieces = ( foo_module.name(&db).clone(), foo_module.file(&db), @@ -2022,7 +2239,7 @@ mod tests { // Re-query the foo module. The foo module should still be cached // because `bar.py` isn't relevant for resolving `foo`. - let foo_module2 = resolve_module(&db, &foo_module_name); + let foo_module2 = resolve_module_confident(&db, &foo_module_name); let foo_pieces2 = foo_module2.map(|foo_module2| { ( foo_module2.name(&db).clone(), @@ -2049,14 +2266,15 @@ mod tests { let foo_path = src.join("foo.py"); let foo_module_name = ModuleName::new_static("foo").unwrap(); - assert_eq!(resolve_module(&db, &foo_module_name), None); + assert_eq!(resolve_module_confident(&db, &foo_module_name), None); // Now write the foo file db.write_file(&foo_path, "x = 1")?; let foo_file = system_path_to_file(&db, &foo_path).expect("foo.py to exist"); - let foo_module = resolve_module(&db, &foo_module_name).expect("Foo module to resolve"); + let foo_module = + resolve_module_confident(&db, &foo_module_name).expect("Foo module to resolve"); assert_eq!(foo_file, foo_module.file(&db).unwrap()); Ok(()) @@ -2070,7 +2288,8 @@ mod tests { let TestCase { mut db, src, .. } = TestCaseBuilder::new().with_src_files(SRC).build(); let foo_module_name = ModuleName::new_static("foo").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).expect("foo module to exist"); + let foo_module = + resolve_module_confident(&db, &foo_module_name).expect("foo module to exist"); let foo_init_path = src.join("foo/__init__.py"); assert_eq!(&foo_init_path, foo_module.file(&db).unwrap().path(&db)); @@ -2082,7 +2301,8 @@ mod tests { File::sync_path(&mut db, &foo_init_path); File::sync_path(&mut db, foo_init_path.parent().unwrap()); - let foo_module = resolve_module(&db, &foo_module_name).expect("Foo module to resolve"); + let foo_module = + resolve_module_confident(&db, &foo_module_name).expect("Foo module to resolve"); assert_eq!(&src.join("foo.py"), foo_module.file(&db).unwrap().path(&db)); Ok(()) @@ -2108,7 +2328,7 @@ mod tests { let functools_module_name = ModuleName::new_static("functools").unwrap(); let stdlib_functools_path = stdlib.join("functools.pyi"); - let functools_module = resolve_module(&db, &functools_module_name).unwrap(); + let functools_module = resolve_module_confident(&db, &functools_module_name).unwrap(); assert_eq!(functools_module.search_path(&db).unwrap(), &stdlib); assert_eq!( Ok(functools_module.file(&db).unwrap()), @@ -2121,7 +2341,7 @@ mod tests { let site_packages_functools_path = site_packages.join("functools.py"); db.write_file(&site_packages_functools_path, "f: int") .unwrap(); - let functools_module = resolve_module(&db, &functools_module_name).unwrap(); + let functools_module = resolve_module_confident(&db, &functools_module_name).unwrap(); let functools_file = functools_module.file(&db).unwrap(); let functools_search_path = functools_module.search_path(&db).unwrap().clone(); let events = db.take_salsa_events(); @@ -2156,7 +2376,7 @@ mod tests { .build(); let functools_module_name = ModuleName::new_static("functools").unwrap(); - let functools_module = resolve_module(&db, &functools_module_name).unwrap(); + let functools_module = resolve_module_confident(&db, &functools_module_name).unwrap(); assert_eq!(functools_module.search_path(&db).unwrap(), &stdlib); assert_eq!( Ok(functools_module.file(&db).unwrap()), @@ -2167,7 +2387,7 @@ mod tests { // since first-party files take higher priority in module resolution: let src_functools_path = src.join("functools.py"); db.write_file(&src_functools_path, "FOO: int").unwrap(); - let functools_module = resolve_module(&db, &functools_module_name).unwrap(); + let functools_module = resolve_module_confident(&db, &functools_module_name).unwrap(); assert_eq!(functools_module.search_path(&db).unwrap(), &src); assert_eq!( Ok(functools_module.file(&db).unwrap()), @@ -2198,7 +2418,7 @@ mod tests { let functools_module_name = ModuleName::new_static("functools").unwrap(); let src_functools_path = src.join("functools.py"); - let functools_module = resolve_module(&db, &functools_module_name).unwrap(); + let functools_module = resolve_module_confident(&db, &functools_module_name).unwrap(); assert_eq!(functools_module.search_path(&db).unwrap(), &src); assert_eq!( Ok(functools_module.file(&db).unwrap()), @@ -2211,7 +2431,7 @@ mod tests { .remove_file(&src_functools_path) .unwrap(); File::sync_path(&mut db, &src_functools_path); - let functools_module = resolve_module(&db, &functools_module_name).unwrap(); + let functools_module = resolve_module_confident(&db, &functools_module_name).unwrap(); assert_eq!(functools_module.search_path(&db).unwrap(), &stdlib); assert_eq!( Ok(functools_module.file(&db).unwrap()), @@ -2233,8 +2453,8 @@ mod tests { let foo_module_name = ModuleName::new_static("foo").unwrap(); let foo_bar_module_name = ModuleName::new_static("foo.bar").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); - let foo_bar_module = resolve_module(&db, &foo_bar_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); + let foo_bar_module = resolve_module_confident(&db, &foo_bar_module_name).unwrap(); assert_eq!( foo_module.file(&db).unwrap().path(&db), @@ -2262,11 +2482,11 @@ mod tests { // Lines with leading whitespace in `.pth` files do not parse: let foo_module_name = ModuleName::new_static("foo").unwrap(); - assert_eq!(resolve_module(&db, &foo_module_name), None); + assert_eq!(resolve_module_confident(&db, &foo_module_name), None); // Lines with trailing whitespace in `.pth` files do: let bar_module_name = ModuleName::new_static("bar").unwrap(); - let bar_module = resolve_module(&db, &bar_module_name).unwrap(); + let bar_module = resolve_module_confident(&db, &bar_module_name).unwrap(); assert_eq!( bar_module.file(&db).unwrap().path(&db), &FilePath::system("/y/src/bar.py") @@ -2285,7 +2505,7 @@ mod tests { .build(); let foo_module_name = ModuleName::new_static("foo").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); assert_eq!( foo_module.file(&db).unwrap().path(&db), @@ -2333,10 +2553,10 @@ not_a_directory let b_module_name = ModuleName::new_static("b").unwrap(); let spam_module_name = ModuleName::new_static("spam").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); - let a_module = resolve_module(&db, &a_module_name).unwrap(); - let b_module = resolve_module(&db, &b_module_name).unwrap(); - let spam_module = resolve_module(&db, &spam_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); + let a_module = resolve_module_confident(&db, &a_module_name).unwrap(); + let b_module = resolve_module_confident(&db, &b_module_name).unwrap(); + let spam_module = resolve_module_confident(&db, &spam_module_name).unwrap(); assert_eq!( foo_module.file(&db).unwrap().path(&db), @@ -2370,14 +2590,14 @@ not_a_directory let foo_module_name = ModuleName::new_static("foo").unwrap(); let bar_module_name = ModuleName::new_static("bar").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); assert_eq!( foo_module.file(&db).unwrap().path(&db), &FilePath::system("/x/src/foo.py") ); db.clear_salsa_events(); - let bar_module = resolve_module(&db, &bar_module_name).unwrap(); + let bar_module = resolve_module_confident(&db, &bar_module_name).unwrap(); assert_eq!( bar_module.file(&db).unwrap().path(&db), &FilePath::system("/y/src/bar.py") @@ -2407,7 +2627,7 @@ not_a_directory db.write_files(x_directory).unwrap(); let foo_module_name = ModuleName::new_static("foo").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); assert_eq!( foo_module.file(&db).unwrap().path(&db), &FilePath::system("/x/src/foo.py") @@ -2419,7 +2639,7 @@ not_a_directory File::sync_path(&mut db, &site_packages.join("_foo.pth")); - assert_eq!(resolve_module(&db, &foo_module_name), None); + assert_eq!(resolve_module_confident(&db, &foo_module_name), None); } #[test] @@ -2434,7 +2654,7 @@ not_a_directory db.write_files(x_directory).unwrap(); let foo_module_name = ModuleName::new_static("foo").unwrap(); - let foo_module = resolve_module(&db, &foo_module_name).unwrap(); + let foo_module = resolve_module_confident(&db, &foo_module_name).unwrap(); let src_path = SystemPathBuf::from("/x/src"); assert_eq!( foo_module.file(&db).unwrap().path(&db), @@ -2447,7 +2667,7 @@ not_a_directory db.memory_file_system().remove_directory(&src_path).unwrap(); File::sync_path(&mut db, &src_path.join("foo.py")); File::sync_path(&mut db, &src_path); - assert_eq!(resolve_module(&db, &foo_module_name), None); + assert_eq!(resolve_module_confident(&db, &foo_module_name), None); } #[test] @@ -2507,7 +2727,7 @@ not_a_directory // The editable installs discovered from the `.pth` file in the first `site-packages` directory // take precedence over the second `site-packages` directory... let a_module_name = ModuleName::new_static("a").unwrap(); - let a_module = resolve_module(&db, &a_module_name).unwrap(); + let a_module = resolve_module_confident(&db, &a_module_name).unwrap(); assert_eq!( a_module.file(&db).unwrap().path(&db), &editable_install_location @@ -2521,7 +2741,7 @@ not_a_directory // ...But now that the `.pth` file in the first `site-packages` directory has been deleted, // the editable install no longer exists, so the module now resolves to the file in the // second `site-packages` directory - let a_module = resolve_module(&db, &a_module_name).unwrap(); + let a_module = resolve_module_confident(&db, &a_module_name).unwrap(); assert_eq!( a_module.file(&db).unwrap().path(&db), &system_site_packages_location @@ -2579,12 +2799,12 @@ not_a_directory // Now try to resolve the module `A` (note the capital `A` instead of `a`). let a_module_name = ModuleName::new_static("A").unwrap(); - assert_eq!(resolve_module(&db, &a_module_name), None); + assert_eq!(resolve_module_confident(&db, &a_module_name), None); // Now lookup the same module using the lowercase `a` and it should // resolve to the file in the system site-packages let a_module_name = ModuleName::new_static("a").unwrap(); - let a_module = resolve_module(&db, &a_module_name).expect("a.py to resolve"); + let a_module = resolve_module_confident(&db, &a_module_name).expect("a.py to resolve"); assert!( a_module .file(&db) diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index 3cb66efe33..98f7a2b8e4 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -1,7 +1,7 @@ use ruff_db::files::File; use crate::dunder_all::dunder_all_names; -use crate::module_resolver::{KnownModule, file_to_module}; +use crate::module_resolver::{KnownModule, file_to_module, resolve_module_confident}; use crate::semantic_index::definition::{Definition, DefinitionState}; use crate::semantic_index::place::{PlaceExprRef, ScopedPlaceId}; use crate::semantic_index::scope::ScopeId; @@ -14,7 +14,7 @@ use crate::types::{ Truthiness, Type, TypeAndQualifiers, TypeQualifiers, UnionBuilder, UnionType, binding_type, declaration_type, todo_type, }; -use crate::{Db, FxOrderSet, Program, resolve_module}; +use crate::{Db, FxOrderSet, Program}; pub(crate) use implicit_globals::{ module_type_implicit_global_declaration, module_type_implicit_global_symbol, @@ -379,7 +379,7 @@ pub(crate) fn imported_symbol<'db>( /// and should not be used when a symbol is being explicitly imported from the `builtins` module /// (e.g. `from builtins import int`). pub(crate) fn builtins_symbol<'db>(db: &'db dyn Db, symbol: &str) -> PlaceAndQualifiers<'db> { - resolve_module(db, &KnownModule::Builtins.name()) + resolve_module_confident(db, &KnownModule::Builtins.name()) .and_then(|module| { let file = module.file(db)?; Some( @@ -409,7 +409,7 @@ pub(crate) fn known_module_symbol<'db>( known_module: KnownModule, symbol: &str, ) -> PlaceAndQualifiers<'db> { - resolve_module(db, &known_module.name()) + resolve_module_confident(db, &known_module.name()) .and_then(|module| { let file = module.file(db)?; Some(imported_symbol(db, file, symbol, None)) @@ -448,7 +448,7 @@ pub(crate) fn builtins_module_scope(db: &dyn Db) -> Option> { /// /// Can return `None` if a custom typeshed is used that is missing the core module in question. fn core_module_scope(db: &dyn Db, core_module: KnownModule) -> Option> { - let module = resolve_module(db, &core_module.name())?; + let module = resolve_module_confident(db, &core_module.name())?; Some(global_scope(db, module.file(db)?)) } diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 9ba2069784..373c80f7ee 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -1582,7 +1582,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { continue; }; - let Some(module) = resolve_module(self.db, &module_name) else { + let Some(module) = resolve_module(self.db, self.file, &module_name) else { continue; }; diff --git a/crates/ty_python_semantic/src/semantic_index/re_exports.rs b/crates/ty_python_semantic/src/semantic_index/re_exports.rs index 22389fc549..693c0f3437 100644 --- a/crates/ty_python_semantic/src/semantic_index/re_exports.rs +++ b/crates/ty_python_semantic/src/semantic_index/re_exports.rs @@ -250,7 +250,9 @@ impl<'db> Visitor<'db> for ExportFinder<'db> { for export in ModuleName::from_import_statement(self.db, self.file, node) .ok() - .and_then(|module_name| resolve_module(self.db, &module_name)) + .and_then(|module_name| { + resolve_module(self.db, self.file, &module_name) + }) .iter() .flat_map(|module| { module diff --git a/crates/ty_python_semantic/src/semantic_model.rs b/crates/ty_python_semantic/src/semantic_model.rs index 2057db47ab..4dc8a59bab 100644 --- a/crates/ty_python_semantic/src/semantic_model.rs +++ b/crates/ty_python_semantic/src/semantic_model.rs @@ -100,14 +100,14 @@ impl<'db> SemanticModel<'db> { pub fn resolve_module(&self, module: Option<&str>, level: u32) -> Option> { let module_name = ModuleName::from_identifier_parts(self.db, self.file, module, level).ok()?; - resolve_module(self.db, &module_name) + resolve_module(self.db, self.file, &module_name) } /// Returns completions for symbols available in a `import ` context. pub fn import_completions(&self) -> Vec> { let typing_extensions = ModuleName::new("typing_extensions").unwrap(); let is_typing_extensions_available = self.file.is_stub(self.db) - || resolve_real_shadowable_module(self.db, &typing_extensions).is_some(); + || resolve_real_shadowable_module(self.db, self.file, &typing_extensions).is_some(); list_modules(self.db) .into_iter() .filter(|module| { @@ -146,7 +146,7 @@ impl<'db> SemanticModel<'db> { &self, module_name: &ModuleName, ) -> Vec> { - let Some(module) = resolve_module(self.db, module_name) else { + let Some(module) = resolve_module(self.db, self.file, module_name) else { tracing::debug!("Could not resolve module from `{module_name:?}`"); return vec![]; }; @@ -156,7 +156,7 @@ impl<'db> SemanticModel<'db> { /// Returns completions for symbols available in the given module as if /// it were imported by this model's `File`. fn module_completions(&self, module_name: &ModuleName) -> Vec> { - let Some(module) = resolve_module(self.db, module_name) else { + let Some(module) = resolve_module(self.db, self.file, module_name) else { tracing::debug!("Could not resolve module from `{module_name:?}`"); return vec![]; }; diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index e3b0aa7717..b1cfc9d3c9 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -12684,7 +12684,7 @@ impl<'db> ModuleLiteralType<'db> { let relative_submodule_name = ModuleName::new(name)?; let mut absolute_submodule_name = self.module(db).name(db).clone(); absolute_submodule_name.extend(&relative_submodule_name); - let submodule = resolve_module(db, &absolute_submodule_name)?; + let submodule = resolve_module(db, importing_file, &absolute_submodule_name)?; Some(Type::module_literal(db, importing_file, submodule)) } diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 5ebf92fb06..06f91502fc 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -5850,7 +5850,7 @@ impl SlotsKind { mod tests { use super::*; use crate::db::tests::setup_db; - use crate::module_resolver::resolve_module; + use crate::module_resolver::resolve_module_confident; use crate::{PythonVersionSource, PythonVersionWithSource}; use salsa::Setter; use strum::IntoEnumIterator; @@ -5866,7 +5866,8 @@ mod tests { }); for class in KnownClass::iter() { let class_name = class.name(&db); - let class_module = resolve_module(&db, &class.canonical_module(&db).name()).unwrap(); + let class_module = + resolve_module_confident(&db, &class.canonical_module(&db).name()).unwrap(); assert_eq!( KnownClass::try_from_file_and_name( diff --git a/crates/ty_python_semantic/src/types/function.rs b/crates/ty_python_semantic/src/types/function.rs index 4437e09698..f8537c27a7 100644 --- a/crates/ty_python_semantic/src/types/function.rs +++ b/crates/ty_python_semantic/src/types/function.rs @@ -1882,7 +1882,7 @@ impl KnownFunction { let Some(module_name) = ModuleName::new(module_name) else { return; }; - let Some(module) = resolve_module(db, &module_name) else { + let Some(module) = resolve_module(db, file, &module_name) else { return; }; diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index 2c53ea275f..111edcabc5 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -938,7 +938,7 @@ mod resolve_definition { }; // Resolve the module to its file - let Some(resolved_module) = resolve_module(db, &module_name) else { + let Some(resolved_module) = resolve_module(db, file, &module_name) else { return Vec::new(); // Module not found, return empty list }; @@ -1025,7 +1025,7 @@ mod resolve_definition { else { return Vec::new(); }; - let Some(resolved_module) = resolve_module(db, &module_name) else { + let Some(resolved_module) = resolve_module(db, file, &module_name) else { return Vec::new(); }; resolved_module.file(db) @@ -1134,7 +1134,12 @@ mod resolve_definition { // It's definitely a stub, so now rerun module resolution but with stubs disabled. let stub_module = file_to_module(db, stub_file_for_module_lookup)?; trace!("Found stub module: {}", stub_module.name(db)); - let real_module = resolve_real_module(db, stub_module.name(db))?; + // We need to pass an importing file to `resolve_real_module` which is a bit odd + // here because there isn't really an importing file. However this `resolve_real_module` + // can be understood as essentially `import .`, which is also what `file_to_module` is, + // so this is in fact exactly the file we want to consider the importer. + let real_module = + resolve_real_module(db, stub_file_for_module_lookup, stub_module.name(db))?; trace!("Found real module: {}", real_module.name(db)); let real_file = real_module.file(db)?; trace!("Found real file: {}", real_file.path(db)); diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 4d05719ebf..d45b35298d 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -5935,7 +5935,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { ) else { return false; }; - resolve_module(self.db(), &module_name).is_some() + resolve_module(self.db(), self.file(), &module_name).is_some() }) { diagnostic .help("The module can be resolved if the number of leading dots is reduced"); @@ -6172,7 +6172,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } }; - if resolve_module(self.db(), &module_name).is_none() { + if resolve_module(self.db(), self.file(), &module_name).is_none() { self.report_unresolved_import(import_from.into(), module_ref.range(), *level, module); } } @@ -6190,7 +6190,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { return; }; - let Some(module) = resolve_module(self.db(), &module_name) else { + let Some(module) = resolve_module(self.db(), self.file(), &module_name) else { self.add_unknown_declaration_with_binding(alias.into(), definition); return; }; @@ -6375,7 +6375,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { self.add_binding(import_from.into(), definition, |_, _| Type::unknown()); return; }; - let Some(module) = resolve_module(self.db(), &thispackage_name) else { + let Some(module) = resolve_module(self.db(), self.file(), &thispackage_name) else { self.add_binding(import_from.into(), definition, |_, _| Type::unknown()); return; }; @@ -6606,7 +6606,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } fn module_type_from_name(&self, module_name: &ModuleName) -> Option> { - resolve_module(self.db(), module_name) + resolve_module(self.db(), self.file(), module_name) .map(|module| Type::module_literal(self.db(), self.file(), module)) } @@ -9186,7 +9186,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { { let mut maybe_submodule_name = module_name.clone(); maybe_submodule_name.extend(&relative_submodule); - if resolve_module(db, &maybe_submodule_name).is_some() { + if resolve_module(db, self.file(), &maybe_submodule_name).is_some() { if let Some(builder) = self .context .report_lint(&POSSIBLY_MISSING_ATTRIBUTE, attribute) diff --git a/crates/ty_python_semantic/src/types/special_form.rs b/crates/ty_python_semantic/src/types/special_form.rs index 423e1196bd..4d3fa066ed 100644 --- a/crates/ty_python_semantic/src/types/special_form.rs +++ b/crates/ty_python_semantic/src/types/special_form.rs @@ -3,8 +3,7 @@ use super::{ClassType, Type, class::KnownClass}; use crate::db::Db; -use crate::module_resolver::{KnownModule, file_to_module}; -use crate::resolve_module; +use crate::module_resolver::{KnownModule, file_to_module, resolve_module_confident}; use crate::semantic_index::place::ScopedPlaceId; use crate::semantic_index::{FileScopeId, place_table, use_def_map}; use crate::types::TypeDefinition; @@ -544,7 +543,7 @@ impl SpecialFormType { self.definition_modules() .iter() .find_map(|module| { - let file = resolve_module(db, &module.name())?.file(db)?; + let file = resolve_module_confident(db, &module.name())?.file(db)?; let scope = FileScopeId::global().to_scope_id(db, file); let symbol_id = place_table(db, scope).symbol_id(self.name())?; diff --git a/crates/ty_test/src/lib.rs b/crates/ty_test/src/lib.rs index ad4e7ebe4e..feb38bdf66 100644 --- a/crates/ty_test/src/lib.rs +++ b/crates/ty_test/src/lib.rs @@ -21,7 +21,7 @@ use ty_python_semantic::types::{UNDEFINED_REVEAL, check_types}; use ty_python_semantic::{ Module, Program, ProgramSettings, PythonEnvironment, PythonPlatform, PythonVersionSource, PythonVersionWithSource, SearchPath, SearchPathSettings, SysPrefixPathOrigin, list_modules, - resolve_module, + resolve_module_confident, }; mod assertion; @@ -259,7 +259,10 @@ fn run_test( } assert!( - matches!(embedded.lang, "py" | "pyi" | "python" | "text" | "cfg"), + matches!( + embedded.lang, + "py" | "pyi" | "python" | "text" | "cfg" | "pth" + ), "Supported file types are: py (or python), pyi, text, cfg and ignore" ); @@ -296,7 +299,16 @@ fn run_test( full_path = new_path; } - db.write_file(&full_path, &embedded.code).unwrap(); + let temp_string; + let to_write = if embedded.lang == "pth" && !embedded.code.starts_with('/') { + // Make any relative .pths be relative to src_path + temp_string = format!("{src_path}/{}", embedded.code); + &*temp_string + } else { + &*embedded.code + }; + + db.write_file(&full_path, to_write).unwrap(); if !(full_path.starts_with(&src_path) && matches!(embedded.lang, "py" | "python" | "pyi")) @@ -566,7 +578,9 @@ struct ModuleInconsistency<'db> { fn run_module_resolution_consistency_test(db: &db::Db) -> Result<(), Vec>> { let mut errs = vec![]; for from_list in list_modules(db) { - errs.push(match resolve_module(db, from_list.name(db)) { + // TODO: For now list_modules does not partake in desperate module resolution so + // only compare against confident module resolution. + errs.push(match resolve_module_confident(db, from_list.name(db)) { None => ModuleInconsistency { db, from_list,