Support import namespace

Author: MichaReiser

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))
             }
         }
     }

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)
         );
 

crates/ty_ide/src/lib.rs

@@ -188,7 +188,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(),

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: <module 'parent.child.one'>
+reveal_type(two)  # revealed: <module 'parent.child.two'>
 ```
 
 ## Regular package in namespace package
@@ -103,3 +105,42 @@ from foo import x
 
 reveal_type(x)  # revealed: Unknown | Literal["module"]
 ```
+
+## `from` import with namespace package
+
+Regression test for <https://github.com/astral-sh/ty/issues/363>
+
+`google/cloud/pubsub_v1/__init__.py`:
+
+```py
+class PublisherClient: ...
+```
+
+```py
+from google.cloud import pubsub_v1
+
+reveal_type(pubsub_v1.PublisherClient)  # revealed: <class 'PublisherClient'>
+```
+
+## `from` root importing sub-packages
+
+Regresssion test for <https://github.com/astral-sh/ty/issues/375>
+
+`opentelemetry/trace/__init__.py`:
+
+```py
+class Trace: ...
+```
+
+`opentelemetry/metrics/__init__.py`:
+
+```py
+class Metric: ...
+```
+
+```py
+from opentelemetry import trace, metrics
+
+reveal_type(trace)  # revealed: <module 'opentelemetry.trace'>
+reveal_type(metrics)  # revealed: <module 'opentelemetry.metrics'>
+```

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.

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<File> {
+        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<KnownModule> {
-        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<KnownModule>,
+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<KnownModule>,
+    },
+
+    /// 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)]

crates/ty_python_semantic/src/module_resolver/path.rs

@@ -143,6 +143,23 @@ impl ModulePath {
         }
     }
 
+    pub(super) fn to_system_path(&self) -> Option<SystemPathBuf> {
+        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<File> {
         let db = resolver.db.upcast();