diff --git a/crates/ruff_graph/src/resolver.rs b/crates/ruff_graph/src/resolver.rs index 8ac228a6b1..baf43c71c9 100644 --- a/crates/ruff_graph/src/resolver.rs +++ b/crates/ruff_graph/src/resolver.rs @@ -19,19 +19,20 @@ impl<'a> Resolver<'a> { pub(crate) fn resolve(&self, import: CollectedImport) -> Option<&'a FilePath> { match import { CollectedImport::Import(import) => { - resolve_module(self.db, &import).map(|module| module.file().path(self.db)) + let module = resolve_module(self.db, &import)?; + Some(module.file()?.path(self.db)) } CollectedImport::ImportFrom(import) => { // Attempt to resolve the member (e.g., given `from foo import bar`, look for `foo.bar`). let parent = import.parent(); - resolve_module(self.db, &import) - .map(|module| module.file().path(self.db)) - .or_else(|| { - // Attempt to resolve the module (e.g., given `from foo import bar`, look for `foo`). + let module = resolve_module(self.db, &import).or_else(|| { + // Attempt to resolve the module (e.g., given `from foo import bar`, look for `foo`). - resolve_module(self.db, &parent?).map(|module| module.file().path(self.db)) - }) + resolve_module(self.db, &parent?) + })?; + + Some(module.file()?.path(self.db)) } } } diff --git a/crates/ty/tests/file_watching.rs b/crates/ty/tests/file_watching.rs index 45eff5d307..c8f3a186b0 100644 --- a/crates/ty/tests/file_watching.rs +++ b/crates/ty/tests/file_watching.rs @@ -1444,13 +1444,11 @@ mod unix { ) .expect("Expected bar.baz to exist in site-packages."); let baz_project = case.project_path("bar/baz.py"); + let baz_file = baz.file().unwrap(); + assert_eq!(source_text(case.db(), baz_file).as_str(), "def baz(): ..."); assert_eq!( - source_text(case.db(), baz.file()).as_str(), - "def baz(): ..." - ); - assert_eq!( - baz.file().path(case.db()).as_system_path(), + baz_file.path(case.db()).as_system_path(), Some(&*baz_project) ); @@ -1465,7 +1463,7 @@ mod unix { case.apply_changes(changes); assert_eq!( - source_text(case.db(), baz.file()).as_str(), + source_text(case.db(), baz_file).as_str(), "def baz(): print('Version 2')" ); @@ -1478,7 +1476,7 @@ mod unix { case.apply_changes(changes); assert_eq!( - source_text(case.db(), baz.file()).as_str(), + source_text(case.db(), baz_file).as_str(), "def baz(): print('Version 3')" ); @@ -1524,6 +1522,7 @@ mod unix { &ModuleName::new_static("bar.baz").unwrap(), ) .expect("Expected bar.baz to exist in site-packages."); + let baz_file = baz.file().unwrap(); let bar_baz = case.project_path("bar/baz.py"); let patched_bar_baz = case.project_path("patched/bar/baz.py"); @@ -1534,11 +1533,8 @@ mod unix { "def baz(): ..." ); - assert_eq!( - source_text(case.db(), baz.file()).as_str(), - "def baz(): ..." - ); - assert_eq!(baz.file().path(case.db()).as_system_path(), Some(&*bar_baz)); + assert_eq!(source_text(case.db(), baz_file).as_str(), "def baz(): ..."); + assert_eq!(baz_file.path(case.db()).as_system_path(), Some(&*bar_baz)); case.assert_indexed_project_files([patched_bar_baz_file]); @@ -1567,7 +1563,7 @@ mod unix { let patched_baz_text = source_text(case.db(), patched_bar_baz_file); let did_update_patched_baz = patched_baz_text.as_str() == "def baz(): print('Version 2')"; - let bar_baz_text = source_text(case.db(), baz.file()); + let bar_baz_text = source_text(case.db(), baz_file); let did_update_bar_baz = bar_baz_text.as_str() == "def baz(): print('Version 2')"; assert!( @@ -1650,7 +1646,7 @@ mod unix { "def baz(): ..." ); assert_eq!( - baz.file().path(case.db()).as_system_path(), + baz.file().unwrap().path(case.db()).as_system_path(), Some(&*baz_original) ); diff --git a/crates/ty_ide/src/lib.rs b/crates/ty_ide/src/lib.rs index 9d862abac4..fe86113064 100644 --- a/crates/ty_ide/src/lib.rs +++ b/crates/ty_ide/src/lib.rs @@ -187,7 +187,10 @@ impl HasNavigationTargets for Type<'_> { impl HasNavigationTargets for TypeDefinition<'_> { fn navigation_targets(&self, db: &dyn Db) -> NavigationTargets { - let full_range = self.full_range(db.upcast()); + let Some(full_range) = self.full_range(db.upcast()) else { + return NavigationTargets::empty(); + }; + NavigationTargets::single(NavigationTarget { file: full_range.file(), focus_range: self.focus_range(db.upcast()).unwrap_or(full_range).range(), diff --git a/crates/ty_python_semantic/resources/mdtest/import/namespace.md b/crates/ty_python_semantic/resources/mdtest/import/namespace.md index 91dff55c58..3b5f63e981 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/namespace.md +++ b/crates/ty_python_semantic/resources/mdtest/import/namespace.md @@ -29,8 +29,10 @@ import parent.child.two `from.py` ```py -# TODO: This should not be an error -from parent.child import one, two # error: [unresolved-import] +from parent.child import one, two + +reveal_type(one) # revealed: +reveal_type(two) # revealed: ``` ## Regular package in namespace package @@ -105,3 +107,42 @@ reveal_type(x) # revealed: Unknown | Literal["module"] import foo.bar # error: [unresolved-import] ``` + +## `from` import with namespace package + +Regression test for + +`google/cloud/pubsub_v1/__init__.py`: + +```py +class PublisherClient: ... +``` + +```py +from google.cloud import pubsub_v1 + +reveal_type(pubsub_v1.PublisherClient) # revealed: +``` + +## `from` root importing sub-packages + +Regresssion test for + +`opentelemetry/trace/__init__.py`: + +```py +class Trace: ... +``` + +`opentelemetry/metrics/__init__.py`: + +```py +class Metric: ... +``` + +```py +from opentelemetry import trace, metrics + +reveal_type(trace) # revealed: +reveal_type(metrics) # revealed: +``` diff --git a/crates/ty_python_semantic/src/dunder_all.rs b/crates/ty_python_semantic/src/dunder_all.rs index 00a1628b78..4265de2ac6 100644 --- a/crates/ty_python_semantic/src/dunder_all.rs +++ b/crates/ty_python_semantic/src/dunder_all.rs @@ -109,8 +109,10 @@ impl<'db> DunderAllNamesCollector<'db> { else { return false; }; - let Some(module_dunder_all_names) = - dunder_all_names(self.db, module_literal.module(self.db).file()) + let Some(module_dunder_all_names) = module_literal + .module(self.db) + .file() + .and_then(|file| dunder_all_names(self.db, file)) else { // The module either does not have a `__all__` variable or it is invalid. return false; @@ -179,7 +181,7 @@ impl<'db> DunderAllNamesCollector<'db> { let module_name = ModuleName::from_import_statement(self.db, self.file, import_from).ok()?; let module = resolve_module(self.db, &module_name)?; - dunder_all_names(self.db, module.file()) + dunder_all_names(self.db, module.file()?) } /// Infer the type of a standalone expression. diff --git a/crates/ty_python_semantic/src/module_resolver/module.rs b/crates/ty_python_semantic/src/module_resolver/module.rs index c56d16dbdc..8ffd477622 100644 --- a/crates/ty_python_semantic/src/module_resolver/module.rs +++ b/crates/ty_python_semantic/src/module_resolver/module.rs @@ -14,15 +14,16 @@ pub struct Module { } impl Module { - pub(crate) fn new( + pub(crate) fn file_module( name: ModuleName, kind: ModuleKind, search_path: SearchPath, file: File, ) -> Self { let known = KnownModule::try_from_search_path_and_name(&search_path, &name); + Self { - inner: Arc::new(ModuleInner { + inner: Arc::new(ModuleInner::FileModule { name, kind, search_path, @@ -32,19 +33,36 @@ impl Module { } } + pub(crate) fn namespace_package(name: ModuleName) -> Self { + Self { + inner: Arc::new(ModuleInner::NamespacePackage { name }), + } + } + /// The absolute name of the module (e.g. `foo.bar`) pub fn name(&self) -> &ModuleName { - &self.inner.name + match &*self.inner { + ModuleInner::FileModule { name, .. } => name, + ModuleInner::NamespacePackage { name, .. } => name, + } } /// The file to the source code that defines this module - pub fn file(&self) -> File { - self.inner.file + /// + /// This is `None` for namespace packages. + pub fn file(&self) -> Option { + match &*self.inner { + ModuleInner::FileModule { file, .. } => Some(*file), + ModuleInner::NamespacePackage { .. } => None, + } } /// Is this a module that we special-case somehow? If so, which one? pub fn known(&self) -> Option { - self.inner.known + match &*self.inner { + ModuleInner::FileModule { known, .. } => *known, + ModuleInner::NamespacePackage { .. } => None, + } } /// Does this module represent the given known module? @@ -53,13 +71,19 @@ impl Module { } /// The search path from which the module was resolved. - pub(crate) fn search_path(&self) -> &SearchPath { - &self.inner.search_path + pub(crate) fn search_path(&self) -> Option<&SearchPath> { + match &*self.inner { + ModuleInner::FileModule { search_path, .. } => Some(search_path), + ModuleInner::NamespacePackage { .. } => None, + } } /// Determine whether this module is a single-file module or a package pub fn kind(&self) -> ModuleKind { - self.inner.kind + match &*self.inner { + ModuleInner::FileModule { kind, .. } => *kind, + ModuleInner::NamespacePackage { .. } => ModuleKind::Package, + } } } @@ -70,17 +94,26 @@ impl std::fmt::Debug for Module { .field("kind", &self.kind()) .field("file", &self.file()) .field("search_path", &self.search_path()) + .field("known", &self.known()) .finish() } } #[derive(PartialEq, Eq, Hash)] -struct ModuleInner { - name: ModuleName, - kind: ModuleKind, - search_path: SearchPath, - file: File, - known: Option, +enum ModuleInner { + /// A module that resolves to a file (`lib.py` or `package/__init__.py`) + FileModule { + name: ModuleName, + kind: ModuleKind, + search_path: SearchPath, + file: File, + known: Option, + }, + + /// A namespace package. Namespace packages are special because + /// there are multiple possible paths and they have no corresponding + /// code file. + NamespacePackage { name: ModuleName }, } #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)] diff --git a/crates/ty_python_semantic/src/module_resolver/path.rs b/crates/ty_python_semantic/src/module_resolver/path.rs index 7e923eeae5..7410e7a748 100644 --- a/crates/ty_python_semantic/src/module_resolver/path.rs +++ b/crates/ty_python_semantic/src/module_resolver/path.rs @@ -143,6 +143,23 @@ impl ModulePath { } } + pub(super) fn to_system_path(&self) -> Option { + let ModulePath { + search_path, + relative_path, + } = self; + match &*search_path.0 { + SearchPathInner::Extra(search_path) + | SearchPathInner::FirstParty(search_path) + | SearchPathInner::SitePackages(search_path) + | SearchPathInner::Editable(search_path) => Some(search_path.join(relative_path)), + SearchPathInner::StandardLibraryCustom(stdlib_root) => { + Some(stdlib_root.join(relative_path)) + } + SearchPathInner::StandardLibraryVendored(_) => None, + } + } + #[must_use] pub(super) fn to_file(&self, resolver: &ResolverContext) -> Option { let db = resolver.db.upcast(); diff --git a/crates/ty_python_semantic/src/module_resolver/resolver.rs b/crates/ty_python_semantic/src/module_resolver/resolver.rs index 5f4aa1e197..d39edc57ca 100644 --- a/crates/ty_python_semantic/src/module_resolver/resolver.rs +++ b/crates/ty_python_semantic/src/module_resolver/resolver.rs @@ -39,22 +39,24 @@ pub(crate) fn resolve_module_query<'db>( let name = module_name.name(db); let _span = tracing::trace_span!("resolve_module", %name).entered(); - let Some((search_path, resolved_module)) = resolve_name(db, name) else { + let Some(resolved) = resolve_name(db, name) else { tracing::debug!("Module `{name}` not found in search paths"); return None; }; - tracing::trace!( - "Resolved module `{name}` to `{path}`", - path = resolved_module.file.path(db) - ); - - let module = Module::new( - name.clone(), - resolved_module.kind, - search_path, - resolved_module.file, - ); + let module = match resolved { + ResolvedName::FileModule(module) => { + tracing::trace!( + "Resolved module `{name}` to `{path}`", + path = module.file.path(db) + ); + Module::file_module(name.clone(), module.kind, module.search_path, module.file) + } + ResolvedName::NamespacePackage => { + tracing::trace!("Module `{name}` is a namespace package"); + Module::namespace_package(name.clone()) + } + }; Some(module) } @@ -117,8 +119,9 @@ pub(crate) fn file_to_module(db: &dyn Db, file: File) -> Option { // 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_file = module.file()?; - if file.path(db) == module.file().path(db) { + if file.path(db) == module_file.path(db) { Some(module) } else { // This path is for a module with the same name but with a different precedence. For example: @@ -616,7 +619,7 @@ 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) -> Option<(SearchPath, ResolvedModule)> { +fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option { let program = Program::get(db); let python_version = program.python_version(db); let resolver_state = ResolverContext::new(db, python_version); @@ -625,6 +628,7 @@ fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, ResolvedM let name = RelaxedModuleName::new(name); let stub_name = name.to_stub_package(); + let mut is_namespace_package = false; for search_path in search_paths(db) { // When a builtin module is imported, standard module resolution is bypassed: @@ -636,16 +640,19 @@ fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, ResolvedM } if !search_path.is_standard_library() { - match resolve_module_in_search_path(&resolver_state, &stub_name, search_path) { - Ok(resolved_module) => { - if resolved_module.package_kind.is_root() && resolved_module.kind.is_module() { + match resolve_name_in_search_path(&resolver_state, &stub_name, search_path) { + Ok((package_kind, ResolvedName::FileModule(module))) => { + if package_kind.is_root() && module.kind.is_module() { tracing::trace!( "Search path '{search_path} contains a module named `{stub_name}` but a standalone module isn't a valid stub." ); } else { - return Some((search_path.clone(), resolved_module)); + return Some(ResolvedName::FileModule(module)); } } + Ok((_, ResolvedName::NamespacePackage)) => { + is_namespace_package = true; + } Err(PackageKind::Root) => { tracing::trace!( "Search path '{search_path}' contains no stub package named `{stub_name}`." @@ -670,8 +677,13 @@ fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, ResolvedM } } - match resolve_module_in_search_path(&resolver_state, &name, search_path) { - Ok(resolved_module) => return Some((search_path.clone(), resolved_module)), + match resolve_name_in_search_path(&resolver_state, &name, search_path) { + Ok((_, ResolvedName::FileModule(module))) => { + return Some(ResolvedName::FileModule(module)); + } + Ok((_, ResolvedName::NamespacePackage)) => { + is_namespace_package = true; + } Err(kind) => match kind { PackageKind::Root => { tracing::trace!( @@ -693,21 +705,36 @@ fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, ResolvedM } } + if is_namespace_package { + return Some(ResolvedName::NamespacePackage); + } + None } #[derive(Debug)] -struct ResolvedModule { +enum ResolvedName { + /// A module that resolves to a file. + FileModule(ResolvedFileModule), + + /// The module name resolved to a namespace package. + /// + /// For example, `from opentelemetry import trace, metrics` where `opentelemetry` is a namespace package (and `trace` and `metrics` are sub packages). + NamespacePackage, +} + +#[derive(Debug)] +struct ResolvedFileModule { kind: ModuleKind, - package_kind: PackageKind, + search_path: SearchPath, file: File, } -fn resolve_module_in_search_path( +fn resolve_name_in_search_path( context: &ResolverContext, name: &RelaxedModuleName, search_path: &SearchPath, -) -> Result { +) -> Result<(PackageKind, ResolvedName), PackageKind> { let mut components = name.components(); let module_name = components.next_back().unwrap(); @@ -720,22 +747,55 @@ fn resolve_module_in_search_path( // Check for a regular package first (highest priority) package_path.push("__init__"); if let Some(regular_package) = resolve_file_module(&package_path, context) { - return Ok(ResolvedModule { - file: regular_package, - kind: ModuleKind::Package, - package_kind: resolved_package.kind, - }); + return Ok(( + resolved_package.kind, + ResolvedName::FileModule(ResolvedFileModule { + search_path: search_path.clone(), + kind: ModuleKind::Package, + file: regular_package, + }), + )); } // Check for a file module next package_path.pop(); if let Some(file_module) = resolve_file_module(&package_path, context) { - return Ok(ResolvedModule { - file: file_module, - kind: ModuleKind::Module, - package_kind: resolved_package.kind, - }); + return Ok(( + resolved_package.kind, + ResolvedName::FileModule(ResolvedFileModule { + file: file_module, + kind: ModuleKind::Module, + search_path: search_path.clone(), + }), + )); + } + + // Last resort, check if a folder with the given name exists. + // If so, then this is a namespace package. + // We need to skip this check for typeshed because the `resolve_file_module` can also return `None` + // if the `__init__.py` exists but isn't available for the current Python version. + // Let's assume that the `xml` module is only available on Python 3.11+ and we're resolving for Python 3.10: + // * `resolve_file_module("xml/__init__.pyi")` returns `None` even though the file exists but the + // module isn't available for the current Python version. + // * The check here would now return `true` because the `xml` directory exists, resulting + // in a false positive for a namespace package. + // + // Since typeshed doesn't use any namespace packages today (May 2025), simply skip this + // check which also helps performance. If typeshed ever uses namespace packages, ensure that + // this check also takes the `VERSIONS` file into consideration. + if !search_path.is_standard_library() && package_path.is_directory(context) { + if let Some(path) = package_path.to_system_path() { + let system = context.db.system(); + if system.case_sensitivity().is_case_sensitive() + || system.path_exists_case_sensitive( + &path, + package_path.search_path().as_system_path().unwrap(), + ) + { + return Ok((resolved_package.kind, ResolvedName::NamespacePackage)); + } + } } Err(resolved_package.kind) @@ -937,11 +997,11 @@ mod tests { ); assert_eq!("foo", foo_module.name()); - assert_eq!(&src, foo_module.search_path()); + assert_eq!(&src, foo_module.search_path().unwrap()); assert_eq!(ModuleKind::Module, foo_module.kind()); let expected_foo_path = src.join("foo.py"); - assert_eq!(&expected_foo_path, foo_module.file().path(&db)); + assert_eq!(&expected_foo_path, foo_module.file().unwrap().path(&db)); assert_eq!( Some(foo_module), path_to_module(&db, &FilePath::System(expected_foo_path)) @@ -958,7 +1018,10 @@ mod tests { let builtins_module_name = ModuleName::new_static("builtins").unwrap(); let builtins = resolve_module(&db, &builtins_module_name).expect("builtins to resolve"); - assert_eq!(builtins.file().path(&db), &stdlib.join("builtins.pyi")); + assert_eq!( + builtins.file().unwrap().path(&db), + &stdlib.join("builtins.pyi") + ); } #[test] @@ -979,7 +1042,10 @@ mod tests { let builtins_module_name = ModuleName::new_static("builtins").unwrap(); let builtins = resolve_module(&db, &builtins_module_name).expect("builtins to resolve"); - assert_eq!(builtins.file().path(&db), &stdlib.join("builtins.pyi")); + assert_eq!( + builtins.file().unwrap().path(&db), + &stdlib.join("builtins.pyi") + ); } #[test] @@ -1002,11 +1068,14 @@ mod tests { resolve_module(&db, &functools_module_name).as_ref() ); - assert_eq!(&stdlib, functools_module.search_path()); + assert_eq!(&stdlib, functools_module.search_path().unwrap()); assert_eq!(ModuleKind::Module, functools_module.kind()); let expected_functools_path = stdlib.join("functools.pyi"); - assert_eq!(&expected_functools_path, functools_module.file().path(&db)); + assert_eq!( + &expected_functools_path, + functools_module.file().unwrap().path(&db) + ); assert_eq!( Some(functools_module), @@ -1052,7 +1121,7 @@ mod tests { let resolved_module = resolve_module(&db, &module_name).unwrap_or_else(|| { panic!("Expected module {module_name} to exist in the mock stdlib") }); - let search_path = resolved_module.search_path(); + let search_path = resolved_module.search_path().unwrap(); assert_eq!( &stdlib, search_path, "Search path for {module_name} was unexpectedly {search_path:?}" @@ -1148,7 +1217,7 @@ mod tests { let resolved_module = resolve_module(&db, &module_name).unwrap_or_else(|| { panic!("Expected module {module_name} to exist in the mock stdlib") }); - let search_path = resolved_module.search_path(); + let search_path = resolved_module.search_path().unwrap(); assert_eq!( &stdlib, search_path, "Search path for {module_name} was unexpectedly {search_path:?}" @@ -1209,9 +1278,12 @@ mod tests { Some(&functools_module), resolve_module(&db, &functools_module_name).as_ref() ); - assert_eq!(&src, functools_module.search_path()); + assert_eq!(&src, functools_module.search_path().unwrap()); assert_eq!(ModuleKind::Module, functools_module.kind()); - assert_eq!(&src.join("functools.py"), functools_module.file().path(&db)); + assert_eq!( + &src.join("functools.py"), + functools_module.file().unwrap().path(&db) + ); assert_eq!( Some(functools_module), @@ -1230,9 +1302,9 @@ mod tests { let pydoc_data_topics = resolve_module(&db, &pydoc_data_topics_name).unwrap(); assert_eq!("pydoc_data.topics", pydoc_data_topics.name()); - assert_eq!(pydoc_data_topics.search_path(), &stdlib); + assert_eq!(pydoc_data_topics.search_path().unwrap(), &stdlib); assert_eq!( - pydoc_data_topics.file().path(&db), + pydoc_data_topics.file().unwrap().path(&db), &stdlib.join("pydoc_data/topics.pyi") ); } @@ -1247,8 +1319,8 @@ mod tests { let foo_module = resolve_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); assert_eq!("foo", foo_module.name()); - assert_eq!(&src, foo_module.search_path()); - assert_eq!(&foo_path, foo_module.file().path(&db)); + assert_eq!(&src, foo_module.search_path().unwrap()); + assert_eq!(&foo_path, foo_module.file().unwrap().path(&db)); assert_eq!( Some(&foo_module), @@ -1274,8 +1346,8 @@ mod tests { let foo_module = resolve_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); let foo_init_path = src.join("foo/__init__.py"); - assert_eq!(&src, foo_module.search_path()); - assert_eq!(&foo_init_path, foo_module.file().path(&db)); + assert_eq!(&src, foo_module.search_path().unwrap()); + assert_eq!(&foo_init_path, foo_module.file().unwrap().path(&db)); assert_eq!(ModuleKind::Package, foo_module.kind()); assert_eq!( @@ -1297,8 +1369,8 @@ mod tests { let foo = resolve_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); let foo_stub = src.join("foo.pyi"); - assert_eq!(&src, foo.search_path()); - assert_eq!(&foo_stub, foo.file().path(&db)); + assert_eq!(&src, foo.search_path().unwrap()); + assert_eq!(&foo_stub, foo.file().unwrap().path(&db)); assert_eq!(Some(foo), path_to_module(&db, &FilePath::System(foo_stub))); assert_eq!( @@ -1321,8 +1393,8 @@ mod tests { resolve_module(&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()); - assert_eq!(&baz_path, baz_module.file().path(&db)); + assert_eq!(&src, baz_module.search_path().unwrap()); + assert_eq!(&baz_path, baz_module.file().unwrap().path(&db)); assert_eq!( Some(baz_module), @@ -1345,8 +1417,8 @@ mod tests { let foo_module = resolve_module(&db, &ModuleName::new_static("foo").unwrap()).unwrap(); let foo_src_path = src.join("foo.py"); - assert_eq!(&src, foo_module.search_path()); - assert_eq!(&foo_src_path, foo_module.file().path(&db)); + assert_eq!(&src, foo_module.search_path().unwrap()); + assert_eq!(&foo_src_path, foo_module.file().unwrap().path(&db)); assert_eq!( Some(foo_module), path_to_module(&db, &FilePath::System(foo_src_path)) @@ -1413,14 +1485,14 @@ mod tests { assert_ne!(foo_module, bar_module); - assert_eq!(&src, foo_module.search_path()); - assert_eq!(&foo, foo_module.file().path(&db)); + assert_eq!(&src, foo_module.search_path().unwrap()); + assert_eq!(&foo, foo_module.file().unwrap().path(&db)); // `foo` and `bar` shouldn't resolve to the same file - assert_eq!(&src, bar_module.search_path()); - assert_eq!(&bar, bar_module.file().path(&db)); - assert_eq!(&foo, foo_module.file().path(&db)); + assert_eq!(&src, bar_module.search_path().unwrap()); + assert_eq!(&bar, bar_module.file().unwrap().path(&db)); + assert_eq!(&foo, foo_module.file().unwrap().path(&db)); assert_ne!(&foo_module, &bar_module); @@ -1484,7 +1556,7 @@ mod tests { 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"); - assert_eq!(foo_file, foo_module.file()); + assert_eq!(foo_file, foo_module.file().unwrap()); Ok(()) } @@ -1500,7 +1572,7 @@ mod tests { let foo_module = resolve_module(&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().path(&db)); + assert_eq!(&foo_init_path, foo_module.file().unwrap().path(&db)); // Delete `foo/__init__.py` and the `foo` folder. `foo` should now resolve to `foo.py` db.memory_file_system().remove_file(&foo_init_path)?; @@ -1510,7 +1582,7 @@ mod tests { File::sync_path(&mut db, foo_init_path.parent().unwrap()); let foo_module = resolve_module(&db, &foo_module_name).expect("Foo module to resolve"); - assert_eq!(&src.join("foo.py"), foo_module.file().path(&db)); + assert_eq!(&src.join("foo.py"), foo_module.file().unwrap().path(&db)); Ok(()) } @@ -1536,9 +1608,9 @@ mod tests { let stdlib_functools_path = stdlib.join("functools.pyi"); let functools_module = resolve_module(&db, &functools_module_name).unwrap(); - assert_eq!(functools_module.search_path(), &stdlib); + assert_eq!(functools_module.search_path().unwrap(), &stdlib); assert_eq!( - Ok(functools_module.file()), + Ok(functools_module.file().unwrap()), system_path_to_file(&db, &stdlib_functools_path) ); @@ -1556,9 +1628,9 @@ mod tests { ModuleNameIngredient::new(&db, functools_module_name), &events, ); - assert_eq!(functools_module.search_path(), &stdlib); + assert_eq!(functools_module.search_path().unwrap(), &stdlib); assert_eq!( - Ok(functools_module.file()), + Ok(functools_module.file().unwrap()), system_path_to_file(&db, &stdlib_functools_path) ); } @@ -1582,9 +1654,9 @@ mod tests { let functools_module_name = ModuleName::new_static("functools").unwrap(); let functools_module = resolve_module(&db, &functools_module_name).unwrap(); - assert_eq!(functools_module.search_path(), &stdlib); + assert_eq!(functools_module.search_path().unwrap(), &stdlib); assert_eq!( - Ok(functools_module.file()), + Ok(functools_module.file().unwrap()), system_path_to_file(&db, stdlib.join("functools.pyi")) ); @@ -1593,9 +1665,9 @@ mod tests { 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(); - assert_eq!(functools_module.search_path(), &src); + assert_eq!(functools_module.search_path().unwrap(), &src); assert_eq!( - Ok(functools_module.file()), + Ok(functools_module.file().unwrap()), system_path_to_file(&db, &src_functools_path) ); } @@ -1624,9 +1696,9 @@ mod tests { let src_functools_path = src.join("functools.py"); let functools_module = resolve_module(&db, &functools_module_name).unwrap(); - assert_eq!(functools_module.search_path(), &src); + assert_eq!(functools_module.search_path().unwrap(), &src); assert_eq!( - Ok(functools_module.file()), + Ok(functools_module.file().unwrap()), system_path_to_file(&db, &src_functools_path) ); @@ -1637,9 +1709,9 @@ mod tests { .unwrap(); File::sync_path(&mut db, &src_functools_path); let functools_module = resolve_module(&db, &functools_module_name).unwrap(); - assert_eq!(functools_module.search_path(), &stdlib); + assert_eq!(functools_module.search_path().unwrap(), &stdlib); assert_eq!( - Ok(functools_module.file()), + Ok(functools_module.file().unwrap()), system_path_to_file(&db, stdlib.join("functools.pyi")) ); } @@ -1662,11 +1734,11 @@ mod tests { let foo_bar_module = resolve_module(&db, &foo_bar_module_name).unwrap(); assert_eq!( - foo_module.file().path(&db), + foo_module.file().unwrap().path(&db), &FilePath::system("/x/src/foo/__init__.py") ); assert_eq!( - foo_bar_module.file().path(&db), + foo_bar_module.file().unwrap().path(&db), &FilePath::system("/x/src/foo/bar.py") ); } @@ -1693,7 +1765,7 @@ mod tests { let bar_module_name = ModuleName::new_static("bar").unwrap(); let bar_module = resolve_module(&db, &bar_module_name).unwrap(); assert_eq!( - bar_module.file().path(&db), + bar_module.file().unwrap().path(&db), &FilePath::system("/y/src/bar.py") ); } @@ -1713,7 +1785,7 @@ mod tests { let foo_module = resolve_module(&db, &foo_module_name).unwrap(); assert_eq!( - foo_module.file().path(&db), + foo_module.file().unwrap().path(&db), &FilePath::system("/x/y/src/foo.pyi") ); } @@ -1764,13 +1836,19 @@ not_a_directory let spam_module = resolve_module(&db, &spam_module_name).unwrap(); assert_eq!( - foo_module.file().path(&db), + foo_module.file().unwrap().path(&db), &FilePath::system("/x/y/src/foo.pyi") ); - assert_eq!(a_module.file().path(&db), &FilePath::system("/a.py")); - assert_eq!(b_module.file().path(&db), &FilePath::system("/baz/b.py")); assert_eq!( - spam_module.file().path(&db), + a_module.file().unwrap().path(&db), + &FilePath::system("/a.py") + ); + assert_eq!( + b_module.file().unwrap().path(&db), + &FilePath::system("/baz/b.py") + ); + assert_eq!( + spam_module.file().unwrap().path(&db), &FilePath::System(site_packages.join("spam/spam.py")) ); } @@ -1791,14 +1869,14 @@ not_a_directory let foo_module = resolve_module(&db, &foo_module_name).unwrap(); assert_eq!( - foo_module.file().path(&db), + foo_module.file().unwrap().path(&db), &FilePath::system("/x/src/foo.py") ); db.clear_salsa_events(); let bar_module = resolve_module(&db, &bar_module_name).unwrap(); assert_eq!( - bar_module.file().path(&db), + bar_module.file().unwrap().path(&db), &FilePath::system("/y/src/bar.py") ); let events = db.take_salsa_events(); @@ -1823,7 +1901,7 @@ not_a_directory let foo_module_name = ModuleName::new_static("foo").unwrap(); let foo_module = resolve_module(&db, &foo_module_name).unwrap(); assert_eq!( - foo_module.file().path(&db), + foo_module.file().unwrap().path(&db), &FilePath::system("/x/src/foo.py") ); @@ -1851,7 +1929,7 @@ not_a_directory let foo_module = resolve_module(&db, &foo_module_name).unwrap(); let src_path = SystemPathBuf::from("/x/src"); assert_eq!( - foo_module.file().path(&db), + foo_module.file().unwrap().path(&db), &FilePath::System(src_path.join("foo.py")) ); @@ -1925,7 +2003,10 @@ not_a_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(); - assert_eq!(a_module.file().path(&db), &editable_install_location); + assert_eq!( + a_module.file().unwrap().path(&db), + &editable_install_location + ); db.memory_file_system() .remove_file(&site_packages_pth) @@ -1936,7 +2017,10 @@ not_a_directory // 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(); - assert_eq!(a_module.file().path(&db), &system_site_packages_location); + assert_eq!( + a_module.file().unwrap().path(&db), + &system_site_packages_location + ); } #[test] @@ -2001,6 +2085,7 @@ not_a_directory assert!( a_module .file() + .unwrap() .path(&db) .as_str() .ends_with("src/a/__init__.py"), @@ -2035,6 +2120,6 @@ not_a_directory let foo_module_file = File::new(&db, FilePath::System(installed_foo_module)); let module = file_to_module(&db, foo_module_file).unwrap(); - assert_eq!(module.search_path(), &site_packages); + assert_eq!(module.search_path().unwrap(), &site_packages); } } diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index c9328c02aa..514c1f1676 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -1336,7 +1336,9 @@ where continue; }; - let referenced_module = module.file(); + let Some(referenced_module) = module.file() else { + continue; + }; // In order to understand the visibility of definitions created by a `*` import, // we need to know the visibility of the global-scope definitions in the 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 3c3c664bbd..b8002dbf0b 100644 --- a/crates/ty_python_semantic/src/semantic_index/re_exports.rs +++ b/crates/ty_python_semantic/src/semantic_index/re_exports.rs @@ -244,7 +244,12 @@ impl<'db> Visitor<'db> for ExportFinder<'db> { .ok() .and_then(|module_name| resolve_module(self.db, &module_name)) .iter() - .flat_map(|module| exported_names(self.db, module.file())) + .flat_map(|module| { + module + .file() + .map(|file| exported_names(self.db, file)) + .unwrap_or_default() + }) { self.possibly_add_export(export, PossibleExportKind::Normal); } diff --git a/crates/ty_python_semantic/src/symbol.rs b/crates/ty_python_semantic/src/symbol.rs index f490bddcf3..8e20a08954 100644 --- a/crates/ty_python_semantic/src/symbol.rs +++ b/crates/ty_python_semantic/src/symbol.rs @@ -342,19 +342,22 @@ pub(crate) fn imported_symbol<'db>( /// (e.g. `from builtins import int`). pub(crate) fn builtins_symbol<'db>(db: &'db dyn Db, symbol: &str) -> SymbolAndQualifiers<'db> { resolve_module(db, &KnownModule::Builtins.name()) - .map(|module| { - symbol_impl( - db, - global_scope(db, module.file()), - symbol, - RequiresExplicitReExport::Yes, + .and_then(|module| { + let file = module.file()?; + Some( + symbol_impl( + db, + global_scope(db, file), + symbol, + RequiresExplicitReExport::Yes, + ) + .or_fall_back_to(db, || { + // We're looking up in the builtins namespace and not the module, so we should + // do the normal lookup in `types.ModuleType` and not the special one as in + // `imported_symbol`. + module_type_implicit_global_symbol(db, symbol) + }), ) - .or_fall_back_to(db, || { - // We're looking up in the builtins namespace and not the module, so we should - // do the normal lookup in `types.ModuleType` and not the special one as in - // `imported_symbol`. - module_type_implicit_global_symbol(db, symbol) - }) }) .unwrap_or_default() } @@ -368,7 +371,10 @@ pub(crate) fn known_module_symbol<'db>( symbol: &str, ) -> SymbolAndQualifiers<'db> { resolve_module(db, &known_module.name()) - .map(|module| imported_symbol(db, module.file(), symbol, None)) + .and_then(|module| { + let file = module.file()?; + Some(imported_symbol(db, file, symbol, None)) + }) .unwrap_or_default() } @@ -403,7 +409,8 @@ 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> { - resolve_module(db, &core_module.name()).map(|module| global_scope(db, module.file())) + let module = resolve_module(db, &core_module.name())?; + Some(global_scope(db, module.file()?)) } /// Infer the combined type from an iterator of bindings, and return it diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 6533cec7f2..91ec62ea44 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -7980,7 +7980,11 @@ impl<'db> ModuleLiteralType<'db> { } } - imported_symbol(db, self.module(db).file(), name, None).symbol + self.module(db) + .file() + .map(|file| imported_symbol(db, file, name, None)) + .unwrap_or_default() + .symbol } } diff --git a/crates/ty_python_semantic/src/types/call/bind.rs b/crates/ty_python_semantic/src/types/call/bind.rs index 9750bd5304..a94022f9c3 100644 --- a/crates/ty_python_semantic/src/types/call/bind.rs +++ b/crates/ty_python_semantic/src/types/call/bind.rs @@ -611,8 +611,12 @@ impl<'db> Bindings<'db> { if let [Some(ty)] = overload.parameter_types() { overload.set_return_type(match ty { Type::ModuleLiteral(module_literal) => { - match dunder_all_names(db, module_literal.module(db).file()) - { + let all_names = module_literal + .module(db) + .file() + .map(|file| dunder_all_names(db, file)) + .unwrap_or_default(); + match all_names { Some(names) => { let mut names = names.iter().collect::>(); names.sort(); diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 586823f039..22692f5696 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -2898,7 +2898,7 @@ mod tests { let class_module = resolve_module(&db, &class.canonical_module(&db).name()).unwrap(); assert_eq!( - KnownClass::try_from_file_and_name(&db, class_module.file(), class_name), + KnownClass::try_from_file_and_name(&db, class_module.file().unwrap(), class_name), Some(class), "`KnownClass::candidate_from_str` appears to be missing a case for `{class_name}`" ); diff --git a/crates/ty_python_semantic/src/types/definition.rs b/crates/ty_python_semantic/src/types/definition.rs index 207aed39b6..466673b09f 100644 --- a/crates/ty_python_semantic/src/types/definition.rs +++ b/crates/ty_python_semantic/src/types/definition.rs @@ -24,16 +24,17 @@ impl TypeDefinition<'_> { } } - pub fn full_range(&self, db: &dyn Db) -> FileRange { + pub fn full_range(&self, db: &dyn Db) -> Option { match self { Self::Module(module) => { - let source = source_text(db.upcast(), module.file()); - FileRange::new(module.file(), TextRange::up_to(source.text_len())) + let file = module.file()?; + let source = source_text(db.upcast(), file); + Some(FileRange::new(file, TextRange::up_to(source.text_len()))) } Self::Class(definition) | Self::Function(definition) | Self::TypeVar(definition) - | Self::TypeAlias(definition) => definition.full_range(db), + | Self::TypeAlias(definition) => Some(definition.full_range(db)), } } } diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index f843958b64..063eb4706a 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -4049,7 +4049,7 @@ impl<'db> TypeInferenceBuilder<'db> { // (e.g. `from parent import submodule` inside the `parent` module). let import_is_self_referential = module_ty .into_module_literal() - .is_some_and(|module| self.file() == module.module(self.db()).file()); + .is_some_and(|module| Some(self.file()) == module.module(self.db()).file()); // First try loading the requested attribute from the module. if !import_is_self_referential {