From 8784f951bb87245d7b3bef788c5e0401da5a8d6d Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Fri, 31 Oct 2025 15:18:19 -0400 Subject: [PATCH 01/14] cautiously model more from import effects --- crates/ruff_db/src/files.rs | 6 +++ .../mdtest/import/nonstandard_conventions.md | 18 ++++---- .../ty_python_semantic/src/semantic_index.rs | 42 +++++++++++++------ 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/crates/ruff_db/src/files.rs b/crates/ruff_db/src/files.rs index 4d57162c7cfb42..1c322419e0039b 100644 --- a/crates/ruff_db/src/files.rs +++ b/crates/ruff_db/src/files.rs @@ -475,6 +475,12 @@ impl File { self.path(db).as_str().ends_with("__init__.pyi") } + /// Returns `true` if the file is an `__init__.pyi` + pub fn is_package(self, db: &dyn Db) -> bool { + let path = self.path(db).as_str(); + path.ends_with("__init__.pyi") || path.ends_with("__init__.py") + } + pub fn source_type(self, db: &dyn Db) -> PySourceType { match self.path(db) { FilePath::System(path) => path diff --git a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md index 848eaae3875fad..8015a034da6f1c 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md +++ b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md @@ -267,12 +267,11 @@ X: int = 42 ```py import mypackage +reveal_type(mypackage.submodule) # revealed: # TODO: this would be nice to support -# error: "has no member `submodule`" -reveal_type(mypackage.submodule) # revealed: Unknown -# error: "has no member `submodule`" +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `submodule`" +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -336,12 +335,11 @@ X: int = 42 ```py import mypackage +reveal_type(mypackage.submodule) # revealed: # TODO: this would be nice to support -# error: "has no member `submodule`" -reveal_type(mypackage.submodule) # revealed: Unknown -# error: "has no member `submodule`" +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `submodule`" +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -613,9 +611,7 @@ X: int = 42 ```py import mypackage -# TODO: this would be nice to support, as it works at runtime -# error: "has no member `imported`" -reveal_type(mypackage.imported.X) # revealed: Unknown +reveal_type(mypackage.imported.X) # revealed: int ``` ## `from` Import of Other Package's Submodule diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index a654873db382d7..78d492d1c0b1d8 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs @@ -112,9 +112,10 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( let Some(file) = importing_module.file(db) else { return Box::default(); }; - if !file.is_package_stub(db) { + if !file.is_package(db) { return Box::default(); } + let is_stub = file.is_package_stub(db); semantic_index(db, file) .maybe_imported_modules .iter() @@ -126,19 +127,34 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( import.level, ) .ok()?; - // We only actually care if this is a direct submodule of the package - // so this part should actually be exactly the importing module. - let importing_module_name = importing_module.name(db); - if importing_module_name != &submodule { - return None; + + if is_stub { + // We only actually care if this is a direct submodule of the package + // so this part should actually be exactly the importing module. + let importing_module_name = importing_module.name(db); + if importing_module_name != &submodule { + return None; + } + submodule.extend(&ModuleName::new(import.submodule.as_str())?); + // Throw out the result if this doesn't resolve to an actual module. + // This is quite expensive, but we've gone through a lot of hoops to + // get here so it won't happen too much. + resolve_module(db, &submodule)?; + // Return only the relative part + submodule.relative_to(importing_module_name) + } else { + // NO + let importing_module_name = importing_module.name(db); + let relative = submodule.relative_to(importing_module_name); + if let Some(final_part) = ModuleName::new(import.submodule.as_str()) + && let Some(relative) = &relative + { + if relative.components().next() == Some(&final_part) { + return None; + } + } + relative } - submodule.extend(&ModuleName::new(import.submodule.as_str())?); - // Throw out the result if this doesn't resolve to an actual module. - // This is quite expensive, but we've gone through a lot of hoops to - // get here so it won't happen too much. - resolve_module(db, &submodule)?; - // Return only the relative part - submodule.relative_to(importing_module_name) }) .collect() } From f5f957f2755f9ea38864682c058c45e31edd52f4 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Fri, 31 Oct 2025 15:44:05 -0400 Subject: [PATCH 02/14] horrible crimes --- .../src/semantic_index/builder.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 5645fed7d4fe31..e835f3b5fb6696 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -1454,6 +1454,18 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { .record_node_reachability(NodeKey::from_node(node)); let mut found_star = false; + let shadowing_is_even_vaguely_possible = node + .names + .iter() + .filter_map(|alias| { + let path = node.module.as_ref()?; + let module = ModuleName::new(path.as_str())?; + module + .components() + .find(|name| *name == alias.name.as_str())?; + Some(true) + }) + .any(|_| true); for (alias_index, alias) in node.names.iter().enumerate() { if &alias.name == "*" { // The following line maintains the invariant that every AST node that @@ -1565,7 +1577,9 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { }; // If there's no alias or a redundant alias, record this as a potential import of a submodule - if alias.asname.is_none() || is_reexported { + if !shadowing_is_even_vaguely_possible + && (alias.asname.is_none() || is_reexported) + { self.maybe_imported_modules.insert(MaybeModuleImport { level: node.level, from_module: node.module.clone().map(Into::into), From 9bbbf7a0db1c76121e5854604b5b9b9b1ae66fae Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Sun, 2 Nov 2025 13:46:53 -0500 Subject: [PATCH 03/14] extremely aggressive heuristic --- .../mdtest/import/nonstandard_conventions.md | 43 ++++------ .../type_properties/is_equivalent_to.md | 26 ++++-- .../ty_python_semantic/src/semantic_index.rs | 79 +++++++++++++------ .../src/semantic_index/builder.rs | 38 ++++----- 4 files changed, 104 insertions(+), 82 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md index 8015a034da6f1c..b421cb65c26862 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md +++ b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md @@ -178,9 +178,7 @@ X: int = 42 ```py import mypackage -# TODO: this is probably safe to allow, as it's an unambiguous import of a submodule -# error: "has no member `imported`" -reveal_type(mypackage.imported.X) # revealed: Unknown +reveal_type(mypackage.imported.X) # revealed: int ``` ## Import of Direct Submodule in `__init__` (Non-Stub Check) @@ -202,9 +200,7 @@ X: int = 42 ```py import mypackage -# TODO: this is probably safe to allow, as it's an unambiguous import of a submodule -# error: "has no member `imported`" -reveal_type(mypackage.imported.X) # revealed: Unknown +reveal_type(mypackage.imported.X) # revealed: int ``` ## Relative `from` Import of Nested Submodule in `__init__` @@ -234,12 +230,10 @@ X: int = 42 ```py import mypackage -# TODO: this would be nice to allow -# error: "has no member `submodule`" -reveal_type(mypackage.submodule) # revealed: Unknown -# error: "has no member `submodule`" +reveal_type(mypackage.submodule) # revealed: +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `submodule`" +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -302,12 +296,10 @@ X: int = 42 ```py import mypackage -# TODO: this would be nice to support -# error: "has no member `submodule`" -reveal_type(mypackage.submodule) # revealed: Unknown -# error: "has no member `submodule`" +reveal_type(mypackage.submodule) # revealed: +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `submodule`" +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -370,12 +362,10 @@ X: int = 42 ```py import mypackage -# TODO: this would be nice to support, and is probably safe to do as it's unambiguous -# error: "has no member `submodule`" -reveal_type(mypackage.submodule) # revealed: Unknown -# error: "has no member `submodule`" +reveal_type(mypackage.submodule) # revealed: +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `submodule`" +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -403,12 +393,10 @@ X: int = 42 ```py import mypackage -# TODO: this would be nice to support, and is probably safe to do as it's unambiguous -# error: "has no member `submodule`" -reveal_type(mypackage.submodule) # revealed: Unknown -# error: "has no member `submodule`" +reveal_type(mypackage.submodule) # revealed: +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `submodule`" +# error: "has no member `nested`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -588,8 +576,7 @@ X: int = 42 ```py import mypackage -# error: "has no member `imported`" -reveal_type(mypackage.imported.X) # revealed: Unknown +reveal_type(mypackage.imported.X) # revealed: int ``` ## `from` Import of Non-Submodule (Non-Stub Check) diff --git a/crates/ty_python_semantic/resources/mdtest/type_properties/is_equivalent_to.md b/crates/ty_python_semantic/resources/mdtest/type_properties/is_equivalent_to.md index 41c7f562bbbd1d..fe846ee2131360 100644 --- a/crates/ty_python_semantic/resources/mdtest/type_properties/is_equivalent_to.md +++ b/crates/ty_python_semantic/resources/mdtest/type_properties/is_equivalent_to.md @@ -607,23 +607,33 @@ module: `module2.py`: ```py -import importlib -import importlib.abc +import imported +import imported.abc +``` + +`imported/__init__.pyi`: + +```pyi +``` + +`imported/abc.pyi`: + +```pyi ``` `main2.py`: ```py -import importlib -from module2 import importlib as other_importlib +import imported +from module2 import imported as other_imported from ty_extensions import TypeOf, static_assert, is_equivalent_to -# error: [unresolved-attribute] "Module `importlib` has no member `abc`" -reveal_type(importlib.abc) # revealed: Unknown +# error: [unresolved-attribute] "Module `imported` has no member `abc`" +reveal_type(imported.abc) # revealed: Unknown -reveal_type(other_importlib.abc) # revealed: +reveal_type(other_imported.abc) # revealed: -static_assert(not is_equivalent_to(TypeOf[importlib], TypeOf[other_importlib])) +static_assert(not is_equivalent_to(TypeOf[imported], TypeOf[other_imported])) ``` [materializations]: https://typing.python.org/en/latest/spec/glossary.html#term-materialize diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index 78d492d1c0b1d8..99890cc6fc3375 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs @@ -116,7 +116,9 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( return Box::default(); } let is_stub = file.is_package_stub(db); - semantic_index(db, file) + let importing_module_name = importing_module.name(db); + + let sub_imports = semantic_index(db, file) .maybe_imported_modules .iter() .filter_map(|import| { @@ -128,34 +130,65 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( ) .ok()?; - if is_stub { - // We only actually care if this is a direct submodule of the package - // so this part should actually be exactly the importing module. - let importing_module_name = importing_module.name(db); - if importing_module_name != &submodule { + Some((submodule, import.names.clone())) + }) + .chain( + semantic_index(db, file) + .imported_modules + .iter() + .map(|module| (module.clone(), vec![])), + ) + .filter_map(|(module, names)| { + let relative = if importing_module_name == &module { + None + } else { + Some(module.relative_to(importing_module_name)?) + }; + Some((relative, names)) + }) + .collect::>(); + + let created_names = sub_imports + .iter() + .flat_map(|(rel, names)| { + names + .iter() + .map(|(name, alias)| alias.as_ref().unwrap_or(name)) + }) + .collect::>(); + + sub_imports + .iter() + .filter_map(|(rel, names)| { + if let Some(rel) = rel { + // The module we're importing from is a submodule of ourself! + // In this case the only submodule attribute we're introducing in ourself + // is the direct submodule, but we don't want to do that if another from import + // explicitly shadows it + let direct_submodule = Name::from(rel.components().next()?.to_owned()); + if created_names.contains(&direct_submodule) { return None; } - submodule.extend(&ModuleName::new(import.submodule.as_str())?); - // Throw out the result if this doesn't resolve to an actual module. - // This is quite expensive, but we've gone through a lot of hoops to - // get here so it won't happen too much. - resolve_module(db, &submodule)?; - // Return only the relative part - submodule.relative_to(importing_module_name) + Some(vec![ModuleName::new(direct_submodule.as_str())?]) } else { - // NO - let importing_module_name = importing_module.name(db); - let relative = submodule.relative_to(importing_module_name); - if let Some(final_part) = ModuleName::new(import.submodule.as_str()) - && let Some(relative) = &relative - { - if relative.components().next() == Some(&final_part) { + // The module we're importing from is ourselves! + // In this case all imports are presumably of modules, as it "doesn't" + // make sense to write `from . import an_actual_function` + let submodules = names.iter().filter_map(|(name, alias)| { + if alias.is_some() && alias.as_ref() != Some(name) { return None; } - } - relative + let subname = ModuleName::new(name)?; + let mut submodule = importing_module_name.clone(); + submodule.extend(&subname); + resolve_module(db, &submodule)?; + Some(subname) + }); + + Some(submodules.collect()) } }) + .flatten() .collect() } @@ -323,7 +356,7 @@ pub(crate) struct SemanticIndex<'db> { pub(crate) struct MaybeModuleImport { level: u32, from_module: Option, - submodule: Name, + names: Vec<(Name, Option)>, } impl<'db> SemanticIndex<'db> { diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index e835f3b5fb6696..d967e2bf39da28 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -1453,19 +1453,22 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { self.current_use_def_map_mut() .record_node_reachability(NodeKey::from_node(node)); + self.maybe_imported_modules.insert(MaybeModuleImport { + level: node.level, + from_module: node.module.clone().map(Into::into), + names: node + .names + .iter() + .map(|name| { + ( + name.name.clone().into(), + name.asname.clone().map(Into::into), + ) + }) + .collect(), + }); + let mut found_star = false; - let shadowing_is_even_vaguely_possible = node - .names - .iter() - .filter_map(|alias| { - let path = node.module.as_ref()?; - let module = ModuleName::new(path.as_str())?; - module - .components() - .find(|name| *name == alias.name.as_str())?; - Some(true) - }) - .any(|_| true); for (alias_index, alias) in node.names.iter().enumerate() { if &alias.name == "*" { // The following line maintains the invariant that every AST node that @@ -1576,17 +1579,6 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { (&alias.name.id, false) }; - // If there's no alias or a redundant alias, record this as a potential import of a submodule - if !shadowing_is_even_vaguely_possible - && (alias.asname.is_none() || is_reexported) - { - self.maybe_imported_modules.insert(MaybeModuleImport { - level: node.level, - from_module: node.module.clone().map(Into::into), - submodule: alias.name.clone().into(), - }); - } - // Look for imports `from __future__ import annotations`, ignore `as ...` // We intentionally don't enforce the rules about location of `__future__` // imports here, we assume the user's intent was to apply the `__future__` From 7c341dc443f29f52a6a677751f67b71e76118fed Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Sun, 2 Nov 2025 13:49:54 -0500 Subject: [PATCH 04/14] appease clippy --- crates/ty_python_semantic/src/semantic_index.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index 99890cc6fc3375..a23218aff4a211 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs @@ -115,14 +115,14 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( if !file.is_package(db) { return Box::default(); } - let is_stub = file.is_package_stub(db); + // let is_stub = file.is_package_stub(db); let importing_module_name = importing_module.name(db); let sub_imports = semantic_index(db, file) .maybe_imported_modules .iter() .filter_map(|import| { - let mut submodule = ModuleName::from_identifier_parts( + let submodule = ModuleName::from_identifier_parts( db, file, import.from_module.as_deref(), @@ -150,7 +150,7 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( let created_names = sub_imports .iter() - .flat_map(|(rel, names)| { + .flat_map(|(_, names)| { names .iter() .map(|(name, alias)| alias.as_ref().unwrap_or(name)) From b54091a6c11ef3e8cadbf5dda455ab83f2a416e9 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Sun, 2 Nov 2025 15:21:15 -0500 Subject: [PATCH 05/14] star imports no more --- .../ty_python_semantic/src/semantic_index.rs | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index a23218aff4a211..5484feb40857ed 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs @@ -118,7 +118,7 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( // let is_stub = file.is_package_stub(db); let importing_module_name = importing_module.name(db); - let sub_imports = semantic_index(db, file) + let imports = semantic_index(db, file) .maybe_imported_modules .iter() .filter_map(|import| { @@ -138,17 +138,9 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( .iter() .map(|module| (module.clone(), vec![])), ) - .filter_map(|(module, names)| { - let relative = if importing_module_name == &module { - None - } else { - Some(module.relative_to(importing_module_name)?) - }; - Some((relative, names)) - }) .collect::>(); - let created_names = sub_imports + let created_names = imports .iter() .flat_map(|(_, names)| { names @@ -157,10 +149,21 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( }) .collect::>(); - sub_imports + imports .iter() - .filter_map(|(rel, names)| { - if let Some(rel) = rel { + .filter_map(|(module, names)| { + let relative = if importing_module_name == module { + None + } else { + Some(module.relative_to(importing_module_name)?) + }; + + // Don't mess with star imports + if names.iter().any(|(name, _)| name.as_str() == "*") { + return None; + } + + if let Some(rel) = relative { // The module we're importing from is a submodule of ourself! // In this case the only submodule attribute we're introducing in ourself // is the direct submodule, but we don't want to do that if another from import @@ -174,6 +177,8 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( // The module we're importing from is ourselves! // In this case all imports are presumably of modules, as it "doesn't" // make sense to write `from . import an_actual_function` + + // FOR NOW NOT ALLOWED let submodules = names.iter().filter_map(|(name, alias)| { if alias.is_some() && alias.as_ref() != Some(name) { return None; From c6c67e71b7bc36f9032edc7fd41f49d20799b6fd Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Mon, 3 Nov 2025 14:41:10 -0500 Subject: [PATCH 06/14] try v4 approach --- .../mdtest/import/nonstandard_conventions.md | 45 +++++++---- .../ty_python_semantic/src/semantic_index.rs | 7 +- .../src/semantic_index/builder.rs | 21 ++++- .../src/semantic_index/definition.rs | 48 +++++++++-- .../src/types/ide_support.rs | 4 +- .../src/types/infer/builder.rs | 79 +++++++++++++------ 6 files changed, 152 insertions(+), 52 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md index b421cb65c26862..c9b35e6bdf8ba3 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md +++ b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md @@ -121,7 +121,9 @@ Y: int = 47 ```py import mypackage -reveal_type(mypackage.imported.X) # revealed: int +# TODO: this could work and would be nice to have +# error: "has no member `imported`" +reveal_type(mypackage.imported.X) # revealed: Unknown # error: "has no member `fails`" reveal_type(mypackage.fails.Y) # revealed: Unknown ``` @@ -178,7 +180,9 @@ X: int = 42 ```py import mypackage -reveal_type(mypackage.imported.X) # revealed: int +# TODO: this could work and would be nice to have +# error: "has no member `imported`" +reveal_type(mypackage.imported.X) # revealed: Unknown ``` ## Import of Direct Submodule in `__init__` (Non-Stub Check) @@ -200,7 +204,9 @@ X: int = 42 ```py import mypackage -reveal_type(mypackage.imported.X) # revealed: int +# TODO: this could work and would be nice to have +# error: "has no member `imported`" +reveal_type(mypackage.imported.X) # revealed: Unknown ``` ## Relative `from` Import of Nested Submodule in `__init__` @@ -296,10 +302,12 @@ X: int = 42 ```py import mypackage -reveal_type(mypackage.submodule) # revealed: -# error: "has no member `nested`" +# TODO: this could work and would be nice to have +# error: "has no member `submodule`" +reveal_type(mypackage.submodule) # revealed: Unknown +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `nested`" +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -327,11 +335,12 @@ X: int = 42 ```py import mypackage -reveal_type(mypackage.submodule) # revealed: +# error: "has no member `submodule`" +reveal_type(mypackage.submodule) # revealed: Unknown # TODO: this would be nice to support -# error: "has no member `nested`" +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `nested`" +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -362,10 +371,11 @@ X: int = 42 ```py import mypackage -reveal_type(mypackage.submodule) # revealed: -# error: "has no member `nested`" +# error: "has no member `submodule`" +reveal_type(mypackage.submodule) # revealed: Unknown +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `nested`" +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -393,10 +403,11 @@ X: int = 42 ```py import mypackage -reveal_type(mypackage.submodule) # revealed: -# error: "has no member `nested`" +# error: "has no member `submodule`" +reveal_type(mypackage.submodule) # revealed: Unknown +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `nested`" +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -684,8 +695,8 @@ import mypackage from mypackage import imported reveal_type(imported.X) # revealed: int -# error: "has no member `fails`" -reveal_type(imported.fails.Y) # revealed: Unknown +# TODO: uhhhh should this work!? +reveal_type(imported.fails.Y) # revealed: int # error: "has no member `fails`" reveal_type(mypackage.fails.Y) # revealed: Unknown ``` diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index 5484feb40857ed..4401ee431b5b08 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs @@ -105,10 +105,15 @@ pub(crate) fn imported_modules<'db>(db: &'db dyn Db, file: File) -> Arc( db: &'db dyn Db, importing_module: Module<'db>, ) -> Box<[ModuleName]> { + // FIXME(Gankra): temporarily disabled + if true { + return Box::default(); + } let Some(file) = importing_module.file(db) else { return Box::default(); }; @@ -177,8 +182,6 @@ pub(crate) fn imported_relative_submodules_of_stub_package<'db>( // The module we're importing from is ourselves! // In this case all imports are presumably of modules, as it "doesn't" // make sense to write `from . import an_actual_function` - - // FOR NOW NOT ALLOWED let submodules = names.iter().filter_map(|(name, alias)| { if alias.is_some() && alias.as_ref() != Some(name) { return None; diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index d967e2bf39da28..dd4c3ac6f62eb4 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -1453,6 +1453,23 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { self.current_use_def_map_mut() .record_node_reachability(NodeKey::from_node(node)); + if node.level == 1 + && let Some(submodule_raw) = &node.module + && let Some(submodule) = ModuleName::new(submodule_raw.as_str()) + && let Some(direct_submodule) = submodule.components().next() + { + let direct_submodule_name = Name::new(direct_submodule); + let symbol = self.add_symbol(direct_submodule_name); + self.add_definition( + symbol.into(), + ImportFromDefinitionNodeRef { + node, + alias_index: None, + is_reexported: true, + }, + ); + } + self.maybe_imported_modules.insert(MaybeModuleImport { level: node.level, from_module: node.module.clone().map(Into::into), @@ -1576,7 +1593,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { let (symbol_name, is_reexported) = if let Some(asname) = &alias.asname { (&asname.id, asname.id == alias.name.id) } else { - (&alias.name.id, false) + (&alias.name.id, node.level == 1 && node.module.is_none()) }; // Look for imports `from __future__ import annotations`, ignore `as ...` @@ -1593,7 +1610,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { symbol.into(), ImportFromDefinitionNodeRef { node, - alias_index, + alias_index: Some(alias_index), is_reexported, }, ); diff --git a/crates/ty_python_semantic/src/semantic_index/definition.rs b/crates/ty_python_semantic/src/semantic_index/definition.rs index 81af22d31440b5..44b6811be84f80 100644 --- a/crates/ty_python_semantic/src/semantic_index/definition.rs +++ b/crates/ty_python_semantic/src/semantic_index/definition.rs @@ -354,7 +354,7 @@ pub(crate) struct StarImportDefinitionNodeRef<'ast> { #[derive(Copy, Clone, Debug)] pub(crate) struct ImportFromDefinitionNodeRef<'ast> { pub(crate) node: &'ast ast::StmtImportFrom, - pub(crate) alias_index: usize, + pub(crate) alias_index: Option, pub(crate) is_reexported: bool, } @@ -561,7 +561,9 @@ impl<'db> DefinitionNodeRef<'_, 'db> { node, alias_index, is_reexported: _, - }) => (&node.names[alias_index]).into(), + }) => alias_index + .map(|alias_index| (&node.names[alias_index]).into()) + .unwrap_or_else(|| node.into()), // INVARIANT: for an invalid-syntax statement such as `from foo import *, bar, *`, // we only create a `StarImportDefinitionKind` for the *first* `*` alias in the names list. @@ -718,7 +720,7 @@ impl DefinitionKind<'_> { pub(crate) fn target_range(&self, module: &ParsedModuleRef) -> TextRange { match self { DefinitionKind::Import(import) => import.alias(module).range(), - DefinitionKind::ImportFrom(import) => import.alias(module).range(), + DefinitionKind::ImportFrom(import) => import.range(module), DefinitionKind::StarImport(import) => import.alias(module).range(), DefinitionKind::Function(function) => function.node(module).name.range(), DefinitionKind::Class(class) => class.node(module).name.range(), @@ -755,7 +757,7 @@ impl DefinitionKind<'_> { pub(crate) fn full_range(&self, module: &ParsedModuleRef) -> TextRange { match self { DefinitionKind::Import(import) => import.alias(module).range(), - DefinitionKind::ImportFrom(import) => import.alias(module).range(), + DefinitionKind::ImportFrom(import) => import.range(module), DefinitionKind::StarImport(import) => import.import(module).range(), DefinitionKind::Function(function) => function.node(module).range(), DefinitionKind::Class(class) => class.node(module).range(), @@ -974,7 +976,7 @@ impl ImportDefinitionKind { #[derive(Clone, Debug, get_size2::GetSize)] pub struct ImportFromDefinitionKind { node: AstNodeRef, - alias_index: usize, + alias_index: Option, is_reexported: bool, } @@ -983,8 +985,34 @@ impl ImportFromDefinitionKind { self.node.node(module) } - pub(crate) fn alias<'ast>(&self, module: &'ast ParsedModuleRef) -> &'ast ast::Alias { - &self.node.node(module).names[self.alias_index] + pub(crate) fn alias<'ast>(&self, module: &'ast ParsedModuleRef) -> Option<&'ast ast::Alias> { + let index = self.alias_index?; + Some(&self.node.node(module).names[index]) + } + + pub(crate) fn name<'ast>(&self, module: &'ast ParsedModuleRef) -> &'ast str { + let node = self.node.node(module); + if let Some(index) = self.alias_index { + &node.names[index].name + } else { + let module_name = node + .module + .as_ref() + .expect("must have non-empty module name in this path"); + module_name + .split_once('.') + .map(|(first, _)| first) + .unwrap_or(module_name.as_str()) + } + } + + pub(crate) fn range(&self, module: &ParsedModuleRef) -> TextRange { + let node = self.node.node(module); + if let Some(index) = self.alias_index { + node.names[index].range() + } else { + node.range() + } } pub(crate) fn is_reexported(&self) -> bool { @@ -1121,6 +1149,12 @@ impl From<&ast::Alias> for DefinitionNodeKey { } } +impl From<&ast::StmtImportFrom> for DefinitionNodeKey { + fn from(node: &ast::StmtImportFrom) -> Self { + Self(NodeKey::from_node(node)) + } +} + impl From<&ast::StmtFunctionDef> for DefinitionNodeKey { fn from(node: &ast::StmtFunctionDef) -> Self { Self(NodeKey::from_node(node)) diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index 02a93302999e1e..897fcdd72e5a14 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -1279,7 +1279,7 @@ mod resolve_definition { let file = definition.file(db); let module = parsed_module(db, file).load(db); let import_node = import_from_def.import(&module); - let alias = import_from_def.alias(&module); + let name = import_from_def.name(&module); // For `ImportFrom`, we need to resolve the original imported symbol name // (alias.name), not the local alias (symbol_name) @@ -1287,7 +1287,7 @@ mod resolve_definition { db, file, import_node, - &alias.name, + name, visited, alias_resolution, ) diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index cbb2fe8236021d..dc8cacbe6f83bf 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -1203,7 +1203,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { DefinitionKind::StarImport(import) => { self.infer_import_from_definition( import.import(self.module()), - import.alias(self.module()), + Some(import.alias(self.module())), definition, ); } @@ -5315,18 +5315,50 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { fn infer_import_from_definition( &mut self, import_from: &ast::StmtImportFrom, - alias: &ast::Alias, + alias: Option<&ast::Alias>, definition: Definition<'db>, ) { - let Ok(module_name) = - ModuleName::from_import_statement(self.db(), self.file(), import_from) - else { - self.add_unknown_declaration_with_binding(alias.into(), definition); - return; + let def_node = || { + alias + .map(AnyNodeRef::from) + .unwrap_or_else(|| import_from.into()) + }; + let def_name = || { + if let Some(alias) = alias { + alias.name.id.clone() + } else { + let module_name = import_from + .module + .as_ref() + .expect("must have non-empty module name in this path"); + let module_name = module_name + .split_once('.') + .map(|(first, _)| first) + .unwrap_or(module_name.as_str()); + module_name.into() + } + }; + let is_star = alias.map(|alias| &alias.name == "*").unwrap_or(false); + let module_name = if alias.is_some() { + let Ok(module_name) = + ModuleName::from_import_statement(self.db(), self.file(), import_from) + else { + self.add_unknown_declaration_with_binding(def_node(), definition); + return; + }; + module_name + } else { + let Ok(module_name) = + ModuleName::from_identifier_parts(self.db(), self.file(), None, 1) + else { + self.add_unknown_declaration_with_binding(def_node(), definition); + return; + }; + module_name }; let Some(module) = resolve_module(self.db(), &module_name) else { - self.add_unknown_declaration_with_binding(alias.into(), definition); + self.add_unknown_declaration_with_binding(def_node(), definition); return; }; @@ -5337,8 +5369,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .place_table(self.scope().file_scope_id(self.db())) .symbol(star_import.symbol_id()) .name() + .clone() } else { - &alias.name.id + def_name() }; // Avoid looking up attributes on a module if a module imports from itself @@ -5347,6 +5380,11 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .as_module_literal() .is_some_and(|module| Some(self.file()) == module.module(self.db()).file(self.db())); + // This isn't actually introducing a symbol here + if alias.is_none() && !import_is_self_referential { + return; + } + // Although it isn't the runtime semantics, we go to some trouble to prioritize a submodule // over module `__getattr__`, because that's what other type checkers do. let mut from_module_getattr = None; @@ -5356,14 +5394,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { if let PlaceAndQualifiers { place: Place::Defined(ty, _, boundness), qualifiers, - } = module_ty.member(self.db(), name) + } = module_ty.member(self.db(), &name) { - if &alias.name != "*" && boundness == Definedness::PossiblyUndefined { + if !is_star && boundness == Definedness::PossiblyUndefined { // TODO: Consider loading _both_ the attribute and any submodule and unioning them // together if the attribute exists but is possibly-unbound. if let Some(builder) = self .context - .report_lint(&POSSIBLY_MISSING_IMPORT, AnyNodeRef::Alias(alias)) + .report_lint(&POSSIBLY_MISSING_IMPORT, def_node()) { builder.into_diagnostic(format_args!( "Member `{name}` of module `{module_name}` may be missing", @@ -5374,7 +5412,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { from_module_getattr = Some((ty, qualifiers)); } else { self.add_declaration_with_binding( - alias.into(), + def_node(), definition, &DeclaredAndInferredType::MightBeDifferent { declared_ty: TypeAndQualifiers { @@ -5393,7 +5431,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // Evaluate whether `X.Y` would constitute a valid submodule name, // given a `from X import Y` statement. If it is valid, this will be `Some()`; // else, it will be `None`. - let full_submodule_name = ModuleName::new(name).map(|final_part| { + let full_submodule_name = ModuleName::new(&name).map(|final_part| { let mut ret = module_name.clone(); ret.extend(&final_part); ret @@ -5419,7 +5457,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .and_then(|submodule_name| self.module_type_from_name(submodule_name)) { self.add_declaration_with_binding( - alias.into(), + def_node(), definition, &DeclaredAndInferredType::are_the_same_type(submodule_type), ); @@ -5430,7 +5468,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // `__getattr__`. if let Some((ty, qualifiers)) = from_module_getattr { self.add_declaration_with_binding( - alias.into(), + def_node(), definition, &DeclaredAndInferredType::MightBeDifferent { declared_ty: TypeAndQualifiers { @@ -5444,9 +5482,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { return; } - self.add_unknown_declaration_with_binding(alias.into(), definition); + self.add_unknown_declaration_with_binding(def_node(), definition); - if &alias.name == "*" { + if is_star { return; } @@ -5454,10 +5492,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { return; } - let Some(builder) = self - .context - .report_lint(&UNRESOLVED_IMPORT, AnyNodeRef::Alias(alias)) - else { + let Some(builder) = self.context.report_lint(&UNRESOLVED_IMPORT, def_node()) else { return; }; From e8219bf9700187dbdd71af0739b93feb96817d9f Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Mon, 3 Nov 2025 15:03:09 -0500 Subject: [PATCH 07/14] uhhh --- crates/ty_python_semantic/src/types/infer/builder.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index dc8cacbe6f83bf..fb1b56c4ab2c2c 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -5382,6 +5382,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // This isn't actually introducing a symbol here if alias.is_none() && !import_is_self_referential { + self.add_unknown_declaration_with_binding(def_node(), definition); return; } From 4c7b92a570ede3412544696e61663f2913e40f4c Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Mon, 3 Nov 2025 15:25:59 -0500 Subject: [PATCH 08/14] only once --- crates/ty_python_semantic/src/semantic_index/builder.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index dd4c3ac6f62eb4..7fe67a272dcb29 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -114,6 +114,7 @@ pub(super) struct SemanticIndexBuilder<'db, 'ast> { expressions_by_node: FxHashMap>, imported_modules: FxHashSet, maybe_imported_modules: FxHashSet, + seen_submodule_imports: FxHashSet, /// Hashset of all [`FileScopeId`]s that correspond to [generator functions]. /// /// [generator functions]: https://docs.python.org/3/glossary.html#term-generator @@ -151,6 +152,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { definitions_by_node: FxHashMap::default(), expressions_by_node: FxHashMap::default(), + seen_submodule_imports: FxHashSet::default(), maybe_imported_modules: FxHashSet::default(), imported_modules: FxHashSet::default(), generator_functions: FxHashSet::default(), @@ -1457,7 +1459,11 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { && let Some(submodule_raw) = &node.module && let Some(submodule) = ModuleName::new(submodule_raw.as_str()) && let Some(direct_submodule) = submodule.components().next() + && !self.seen_submodule_imports.contains(direct_submodule) { + self.seen_submodule_imports + .insert(direct_submodule.to_owned()); + let direct_submodule_name = Name::new(direct_submodule); let symbol = self.add_symbol(direct_submodule_name); self.add_definition( From 5684c42f9bda825f56c31f2b8c132c08ed8c8420 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Mon, 3 Nov 2025 15:45:26 -0500 Subject: [PATCH 09/14] new tests --- .../mdtest/import/nonstandard_conventions.md | 157 ++++++++++++++++++ 1 file changed, 157 insertions(+) diff --git a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md index c9b35e6bdf8ba3..17e559645f0e99 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md +++ b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md @@ -816,3 +816,160 @@ from mypackage import funcmod x = funcmod(1) ``` + +## Shadowing Import With Definition + +`mypackage/__init__.pyi`: + +```pyi +from .funcmod import other + +def funcmod(x: int) -> int: ... +``` + +`mypackage/funcmod/__init__.pyi`: + +```pyi +def other(int) -> int: ... +``` + +`main.py`: + +```py +from mypackage import funcmod + +x = funcmod(1) +``` + +## Shadowing Import With Definition (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from .funcmod import other + +def funcmod(x: int) -> int: + return x +``` + +`mypackage/funcmod/__init__.py`: + +```py +def other(x: int) -> int: + return x +``` + +`main.py`: + +```py +from mypackage import funcmod + +x = funcmod(1) +``` + +## Shadowing Import With Assignment + +`mypackage/__init__.pyi`: + +```pyi +from .funcmod import other + +# TODO: do better! +# error: [invalid-assignment] +funcmod = other +``` + +`mypackage/funcmod/__init__.pyi`: + +```pyi +def other(x: int) -> int: ... +``` + +`main.py`: + +```py +from mypackage import funcmod + +# TODO: do better! +# error: [call-non-callable] +x = funcmod(1) +``` + +## Shadowing Import With Assignment (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from .funcmod import other + +# TODO: do better! +# error: [invalid-assignment] +funcmod = other +``` + +`mypackage/funcmod/__init__.py`: + +```py +def other(x: int) -> int: + return x +``` + +`main.py`: + +```py +from mypackage import funcmod + +# error: [call-non-callable] +x = funcmod(1) +``` + +## Shadowing Import Not Clobbered By Second Import + +`mypackage/__init__.pyi`: + +```pyi +from .funcmod import funcmod as funcmod +from .funcmod import other +``` + +`mypackage/funcmod/__init__.pyi`: + +```pyi +def other(x: int) -> int: ... +def funcmod(x: int) -> int: ... +``` + +`main.py`: + +```py +from mypackage import funcmod + +x = funcmod(1) +``` + +## Shadowing Import Not Clobbered By Second Import (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from .funcmod import funcmod +from .funcmod import other +``` + +`mypackage/funcmod/__init__.py`: + +```py +def other(x: int) -> int: + return x + +def funcmod(x: int) -> int: + return x +``` + +`main.py`: + +```py +from mypackage import funcmod + +x = funcmod(1) +``` From 12f67f4005b53426af1509a425520597fd3a3d95 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Mon, 3 Nov 2025 16:46:40 -0500 Subject: [PATCH 10/14] only in packages --- .../mdtest/import/nonstandard_conventions.md | 21 +++++++++++++++++++ .../src/semantic_index/builder.rs | 1 + .../src/types/infer/builder.rs | 2 +- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md index 17e559645f0e99..c38fa5988ed4b4 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md +++ b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md @@ -817,6 +817,27 @@ from mypackage import funcmod x = funcmod(1) ``` +## Re-export Nameclash Problems In Functions (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from .funcmod import funcmod + +funcmod(1) + +def run(): + # error: [call-non-callable] + funcmod(2) +``` + +`mypackage/funcmod.py`: + +```py +def funcmod(x: int) -> int: + return x +``` + ## Shadowing Import With Definition `mypackage/__init__.pyi`: diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index 7fe67a272dcb29..dfadbf70315d78 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -1459,6 +1459,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { && let Some(submodule_raw) = &node.module && let Some(submodule) = ModuleName::new(submodule_raw.as_str()) && let Some(direct_submodule) = submodule.components().next() + && self.file.is_package(self.db) && !self.seen_submodule_imports.contains(direct_submodule) { self.seen_submodule_imports diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index fb1b56c4ab2c2c..a1276427a24150 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -5385,7 +5385,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { self.add_unknown_declaration_with_binding(def_node(), definition); return; } - + // Although it isn't the runtime semantics, we go to some trouble to prioritize a submodule // over module `__getattr__`, because that's what other type checkers do. let mut from_module_getattr = None; From e1ba332efb44faecf88ae5159858e9af96520b76 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Mon, 3 Nov 2025 22:12:47 -0500 Subject: [PATCH 11/14] proper impl --- .../mdtest/import/nonstandard_conventions.md | 8 - .../src/semantic_index/builder.rs | 11 +- .../src/semantic_index/definition.rs | 88 +++++---- .../src/types/ide_support.rs | 3 +- .../src/types/infer/builder.rs | 169 ++++++++++++------ 5 files changed, 168 insertions(+), 111 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md index c38fa5988ed4b4..fbd3b1c931933e 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md +++ b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md @@ -827,7 +827,6 @@ from .funcmod import funcmod funcmod(1) def run(): - # error: [call-non-callable] funcmod(2) ``` @@ -895,8 +894,6 @@ x = funcmod(1) ```pyi from .funcmod import other -# TODO: do better! -# error: [invalid-assignment] funcmod = other ``` @@ -911,8 +908,6 @@ def other(x: int) -> int: ... ```py from mypackage import funcmod -# TODO: do better! -# error: [call-non-callable] x = funcmod(1) ``` @@ -923,8 +918,6 @@ x = funcmod(1) ```py from .funcmod import other -# TODO: do better! -# error: [invalid-assignment] funcmod = other ``` @@ -940,7 +933,6 @@ def other(x: int) -> int: ```py from mypackage import funcmod -# error: [call-non-callable] x = funcmod(1) ``` diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index dfadbf70315d78..c37f78af138d18 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -26,8 +26,8 @@ use crate::semantic_index::definition::{ AnnotatedAssignmentDefinitionNodeRef, AssignmentDefinitionNodeRef, ComprehensionDefinitionNodeRef, Definition, DefinitionCategory, DefinitionNodeKey, DefinitionNodeRef, Definitions, ExceptHandlerDefinitionNodeRef, ForStmtDefinitionNodeRef, - ImportDefinitionNodeRef, ImportFromDefinitionNodeRef, MatchPatternDefinitionNodeRef, - StarImportDefinitionNodeRef, WithItemDefinitionNodeRef, + ImportDefinitionNodeRef, ImportFromDefinitionNodeRef, ImportFromImplicitDefinitionNodeRef, + MatchPatternDefinitionNodeRef, StarImportDefinitionNodeRef, WithItemDefinitionNodeRef, }; use crate::semantic_index::expression::{Expression, ExpressionKind}; use crate::semantic_index::place::{PlaceExpr, PlaceTableBuilder, ScopedPlaceId}; @@ -1469,10 +1469,9 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { let symbol = self.add_symbol(direct_submodule_name); self.add_definition( symbol.into(), - ImportFromDefinitionNodeRef { + ImportFromImplicitDefinitionNodeRef { node, - alias_index: None, - is_reexported: true, + direct_submodule_name: submodule_raw, }, ); } @@ -1617,7 +1616,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { symbol.into(), ImportFromDefinitionNodeRef { node, - alias_index: Some(alias_index), + alias_index, is_reexported, }, ); diff --git a/crates/ty_python_semantic/src/semantic_index/definition.rs b/crates/ty_python_semantic/src/semantic_index/definition.rs index 44b6811be84f80..94525ddb3d76be 100644 --- a/crates/ty_python_semantic/src/semantic_index/definition.rs +++ b/crates/ty_python_semantic/src/semantic_index/definition.rs @@ -3,6 +3,7 @@ use std::ops::Deref; use ruff_db::files::{File, FileRange}; use ruff_db::parsed::{ParsedModuleRef, parsed_module}; use ruff_python_ast as ast; +use ruff_python_ast::name::Name; use ruff_text_size::{Ranged, TextRange}; use crate::Db; @@ -209,6 +210,7 @@ impl<'db> DefinitionState<'db> { pub(crate) enum DefinitionNodeRef<'ast, 'db> { Import(ImportDefinitionNodeRef<'ast>), ImportFrom(ImportFromDefinitionNodeRef<'ast>), + ImportFromImplicit(ImportFromImplicitDefinitionNodeRef<'ast>), ImportStar(StarImportDefinitionNodeRef<'ast>), For(ForStmtDefinitionNodeRef<'ast, 'db>), Function(&'ast ast::StmtFunctionDef), @@ -290,6 +292,12 @@ impl<'ast> From> for DefinitionNodeRef<'ast, ' } } +impl<'ast> From> for DefinitionNodeRef<'ast, '_> { + fn from(node_ref: ImportFromImplicitDefinitionNodeRef<'ast>) -> Self { + Self::ImportFromImplicit(node_ref) + } +} + impl<'ast, 'db> From> for DefinitionNodeRef<'ast, 'db> { fn from(value: ForStmtDefinitionNodeRef<'ast, 'db>) -> Self { Self::For(value) @@ -354,10 +362,14 @@ pub(crate) struct StarImportDefinitionNodeRef<'ast> { #[derive(Copy, Clone, Debug)] pub(crate) struct ImportFromDefinitionNodeRef<'ast> { pub(crate) node: &'ast ast::StmtImportFrom, - pub(crate) alias_index: Option, + pub(crate) alias_index: usize, pub(crate) is_reexported: bool, } - +#[derive(Copy, Clone, Debug)] +pub(crate) struct ImportFromImplicitDefinitionNodeRef<'ast> { + pub(crate) node: &'ast ast::StmtImportFrom, + pub(crate) direct_submodule_name: &'ast ast::Identifier, +} #[derive(Copy, Clone, Debug)] pub(crate) struct AssignmentDefinitionNodeRef<'ast, 'db> { pub(crate) unpack: Option<(UnpackPosition, Unpack<'db>)>, @@ -427,7 +439,6 @@ impl<'db> DefinitionNodeRef<'_, 'db> { alias_index, is_reexported, }), - DefinitionNodeRef::ImportFrom(ImportFromDefinitionNodeRef { node, alias_index, @@ -437,6 +448,13 @@ impl<'db> DefinitionNodeRef<'_, 'db> { alias_index, is_reexported, }), + DefinitionNodeRef::ImportFromImplicit(ImportFromImplicitDefinitionNodeRef { + node, + direct_submodule_name, + }) => DefinitionKind::ImportFromImplicit(ImportFromImplicitDefinitionKind { + node: AstNodeRef::new(parsed, node), + direct_submodule_name: direct_submodule_name.as_str().into(), + }), DefinitionNodeRef::ImportStar(star_import) => { let StarImportDefinitionNodeRef { node, symbol_id } = star_import; DefinitionKind::StarImport(StarImportDefinitionKind { @@ -561,10 +579,11 @@ impl<'db> DefinitionNodeRef<'_, 'db> { node, alias_index, is_reexported: _, - }) => alias_index - .map(|alias_index| (&node.names[alias_index]).into()) - .unwrap_or_else(|| node.into()), - + }) => (&node.names[alias_index]).into(), + Self::ImportFromImplicit(ImportFromImplicitDefinitionNodeRef { + node, + direct_submodule_name: _, + }) => node.into(), // INVARIANT: for an invalid-syntax statement such as `from foo import *, bar, *`, // we only create a `StarImportDefinitionKind` for the *first* `*` alias in the names list. Self::ImportStar(StarImportDefinitionNodeRef { node, symbol_id: _ }) => node @@ -663,6 +682,7 @@ impl DefinitionCategory { pub enum DefinitionKind<'db> { Import(ImportDefinitionKind), ImportFrom(ImportFromDefinitionKind), + ImportFromImplicit(ImportFromImplicitDefinitionKind), StarImport(StarImportDefinitionKind), Function(AstNodeRef), Class(AstNodeRef), @@ -720,7 +740,8 @@ impl DefinitionKind<'_> { pub(crate) fn target_range(&self, module: &ParsedModuleRef) -> TextRange { match self { DefinitionKind::Import(import) => import.alias(module).range(), - DefinitionKind::ImportFrom(import) => import.range(module), + DefinitionKind::ImportFrom(import) => import.alias(module).range(), + DefinitionKind::ImportFromImplicit(import) => import.import(module).range(), DefinitionKind::StarImport(import) => import.alias(module).range(), DefinitionKind::Function(function) => function.node(module).name.range(), DefinitionKind::Class(class) => class.node(module).name.range(), @@ -757,7 +778,8 @@ impl DefinitionKind<'_> { pub(crate) fn full_range(&self, module: &ParsedModuleRef) -> TextRange { match self { DefinitionKind::Import(import) => import.alias(module).range(), - DefinitionKind::ImportFrom(import) => import.range(module), + DefinitionKind::ImportFrom(import) => import.alias(module).range(), + DefinitionKind::ImportFromImplicit(import) => import.import(module).range(), DefinitionKind::StarImport(import) => import.import(module).range(), DefinitionKind::Function(function) => function.node(module).range(), DefinitionKind::Class(class) => class.node(module).range(), @@ -848,6 +870,7 @@ impl DefinitionKind<'_> { | DefinitionKind::Comprehension(_) | DefinitionKind::WithItem(_) | DefinitionKind::MatchPattern(_) + | DefinitionKind::ImportFromImplicit(_) | DefinitionKind::ExceptHandler(_) => DefinitionCategory::Binding, } } @@ -976,7 +999,7 @@ impl ImportDefinitionKind { #[derive(Clone, Debug, get_size2::GetSize)] pub struct ImportFromDefinitionKind { node: AstNodeRef, - alias_index: Option, + alias_index: usize, is_reexported: bool, } @@ -985,40 +1008,29 @@ impl ImportFromDefinitionKind { self.node.node(module) } - pub(crate) fn alias<'ast>(&self, module: &'ast ParsedModuleRef) -> Option<&'ast ast::Alias> { - let index = self.alias_index?; - Some(&self.node.node(module).names[index]) - } - - pub(crate) fn name<'ast>(&self, module: &'ast ParsedModuleRef) -> &'ast str { - let node = self.node.node(module); - if let Some(index) = self.alias_index { - &node.names[index].name - } else { - let module_name = node - .module - .as_ref() - .expect("must have non-empty module name in this path"); - module_name - .split_once('.') - .map(|(first, _)| first) - .unwrap_or(module_name.as_str()) - } - } - - pub(crate) fn range(&self, module: &ParsedModuleRef) -> TextRange { - let node = self.node.node(module); - if let Some(index) = self.alias_index { - node.names[index].range() - } else { - node.range() - } + pub(crate) fn alias<'ast>(&self, module: &'ast ParsedModuleRef) -> &'ast ast::Alias { + &self.node.node(module).names[self.alias_index] } pub(crate) fn is_reexported(&self) -> bool { self.is_reexported } } +#[derive(Clone, Debug, get_size2::GetSize)] +pub struct ImportFromImplicitDefinitionKind { + node: AstNodeRef, + direct_submodule_name: Name, +} + +impl ImportFromImplicitDefinitionKind { + pub fn import<'ast>(&self, module: &'ast ParsedModuleRef) -> &'ast ast::StmtImportFrom { + self.node.node(module) + } + + pub(crate) fn direct_submodule_name(&self) -> &Name { + &self.direct_submodule_name + } +} #[derive(Clone, Debug, get_size2::GetSize)] pub struct AssignmentDefinitionKind<'db> { diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index 897fcdd72e5a14..d6f4d08f139432 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -1279,7 +1279,7 @@ mod resolve_definition { let file = definition.file(db); let module = parsed_module(db, file).load(db); let import_node = import_from_def.import(&module); - let name = import_from_def.name(&module); + let name = &import_from_def.alias(&module).name; // For `ImportFrom`, we need to resolve the original imported symbol name // (alias.name), not the local alias (symbol_name) @@ -1619,6 +1619,7 @@ mod resolve_definition { DefinitionKind::TypeAlias(_) | DefinitionKind::Import(_) | DefinitionKind::ImportFrom(_) + | DefinitionKind::ImportFromImplicit(_) | DefinitionKind::StarImport(_) | DefinitionKind::NamedExpression(_) | DefinitionKind::Assignment(_) diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index a1276427a24150..6a04fa8f643f97 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -4,6 +4,7 @@ use itertools::{Either, Itertools}; use ruff_db::diagnostic::{Annotation, DiagnosticId, Severity}; use ruff_db::files::File; use ruff_db::parsed::ParsedModuleRef; +use ruff_python_ast::name::Name; use ruff_python_ast::visitor::{Visitor, walk_expr}; use ruff_python_ast::{self as ast, AnyNodeRef, ExprContext, PythonVersion}; use ruff_python_stdlib::builtins::version_builtin_was_added; @@ -1200,10 +1201,17 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { definition, ); } + DefinitionKind::ImportFromImplicit(import_from) => { + self.infer_import_from_implicit_definition( + import_from.import(self.module()), + import_from.direct_submodule_name(), + definition, + ); + } DefinitionKind::StarImport(import) => { self.infer_import_from_definition( import.import(self.module()), - Some(import.alias(self.module())), + import.alias(self.module()), definition, ); } @@ -5315,50 +5323,18 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { fn infer_import_from_definition( &mut self, import_from: &ast::StmtImportFrom, - alias: Option<&ast::Alias>, + alias: &ast::Alias, definition: Definition<'db>, ) { - let def_node = || { - alias - .map(AnyNodeRef::from) - .unwrap_or_else(|| import_from.into()) - }; - let def_name = || { - if let Some(alias) = alias { - alias.name.id.clone() - } else { - let module_name = import_from - .module - .as_ref() - .expect("must have non-empty module name in this path"); - let module_name = module_name - .split_once('.') - .map(|(first, _)| first) - .unwrap_or(module_name.as_str()); - module_name.into() - } - }; - let is_star = alias.map(|alias| &alias.name == "*").unwrap_or(false); - let module_name = if alias.is_some() { - let Ok(module_name) = - ModuleName::from_import_statement(self.db(), self.file(), import_from) - else { - self.add_unknown_declaration_with_binding(def_node(), definition); - return; - }; - module_name - } else { - let Ok(module_name) = - ModuleName::from_identifier_parts(self.db(), self.file(), None, 1) - else { - self.add_unknown_declaration_with_binding(def_node(), definition); - return; - }; - module_name + let Ok(module_name) = + ModuleName::from_import_statement(self.db(), self.file(), import_from) + else { + self.add_unknown_declaration_with_binding(alias.into(), definition); + return; }; let Some(module) = resolve_module(self.db(), &module_name) else { - self.add_unknown_declaration_with_binding(def_node(), definition); + self.add_unknown_declaration_with_binding(alias.into(), definition); return; }; @@ -5369,9 +5345,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .place_table(self.scope().file_scope_id(self.db())) .symbol(star_import.symbol_id()) .name() - .clone() } else { - def_name() + &alias.name.id }; // Avoid looking up attributes on a module if a module imports from itself @@ -5380,12 +5355,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .as_module_literal() .is_some_and(|module| Some(self.file()) == module.module(self.db()).file(self.db())); - // This isn't actually introducing a symbol here - if alias.is_none() && !import_is_self_referential { - self.add_unknown_declaration_with_binding(def_node(), definition); - return; - } - // Although it isn't the runtime semantics, we go to some trouble to prioritize a submodule // over module `__getattr__`, because that's what other type checkers do. let mut from_module_getattr = None; @@ -5395,14 +5364,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { if let PlaceAndQualifiers { place: Place::Defined(ty, _, boundness), qualifiers, - } = module_ty.member(self.db(), &name) + } = module_ty.member(self.db(), name) { - if !is_star && boundness == Definedness::PossiblyUndefined { + if &alias.name != "*" && boundness == Definedness::PossiblyUndefined { // TODO: Consider loading _both_ the attribute and any submodule and unioning them // together if the attribute exists but is possibly-unbound. if let Some(builder) = self .context - .report_lint(&POSSIBLY_MISSING_IMPORT, def_node()) + .report_lint(&POSSIBLY_MISSING_IMPORT, AnyNodeRef::Alias(alias)) { builder.into_diagnostic(format_args!( "Member `{name}` of module `{module_name}` may be missing", @@ -5413,7 +5382,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { from_module_getattr = Some((ty, qualifiers)); } else { self.add_declaration_with_binding( - def_node(), + alias.into(), definition, &DeclaredAndInferredType::MightBeDifferent { declared_ty: TypeAndQualifiers { @@ -5432,7 +5401,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // Evaluate whether `X.Y` would constitute a valid submodule name, // given a `from X import Y` statement. If it is valid, this will be `Some()`; // else, it will be `None`. - let full_submodule_name = ModuleName::new(&name).map(|final_part| { + let full_submodule_name = ModuleName::new(name).map(|final_part| { let mut ret = module_name.clone(); ret.extend(&final_part); ret @@ -5458,7 +5427,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .and_then(|submodule_name| self.module_type_from_name(submodule_name)) { self.add_declaration_with_binding( - def_node(), + alias.into(), definition, &DeclaredAndInferredType::are_the_same_type(submodule_type), ); @@ -5469,7 +5438,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // `__getattr__`. if let Some((ty, qualifiers)) = from_module_getattr { self.add_declaration_with_binding( - def_node(), + alias.into(), definition, &DeclaredAndInferredType::MightBeDifferent { declared_ty: TypeAndQualifiers { @@ -5483,9 +5452,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { return; } - self.add_unknown_declaration_with_binding(def_node(), definition); + self.add_unknown_declaration_with_binding(alias.into(), definition); - if is_star { + if &alias.name == "*" { return; } @@ -5493,7 +5462,91 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { return; } - let Some(builder) = self.context.report_lint(&UNRESOLVED_IMPORT, def_node()) else { + let Some(builder) = self + .context + .report_lint(&UNRESOLVED_IMPORT, AnyNodeRef::Alias(alias)) + else { + return; + }; + + let diagnostic = builder.into_diagnostic(format_args!( + "Module `{module_name}` has no member `{name}`" + )); + + if let Some(full_submodule_name) = full_submodule_name { + hint_if_stdlib_submodule_exists_on_other_versions( + self.db(), + diagnostic, + &full_submodule_name, + module, + ); + } + } + + fn infer_import_from_implicit_definition( + &mut self, + import_from: &ast::StmtImportFrom, + direct_submodule_name: &Name, + definition: Definition<'db>, + ) { + let name = direct_submodule_name + .split_once('.') + .map(|(first, _)| first) + .unwrap_or(direct_submodule_name.as_str()); + let Ok(module_name) = ModuleName::from_identifier_parts(self.db(), self.file(), None, 1) + else { + self.add_binding(import_from.into(), definition, |_, _| Type::unknown()); + return; + }; + + let Some(module) = resolve_module(self.db(), &module_name) else { + self.add_binding(import_from.into(), definition, |_, _| Type::unknown()); + return; + }; + + // Evaluate whether `X.Y` would constitute a valid submodule name, + // given a `from X import Y` statement. If it is valid, this will be `Some()`; + // else, it will be `None`. + + let full_submodule_name = ModuleName::new(name).map(|final_part| { + let mut ret = module_name.clone(); + ret.extend(&final_part); + ret + }); + + // If the module doesn't bind the symbol, check if it's a submodule. This won't get + // handled by the `Type::member` call because it relies on the semantic index's + // `imported_modules` set. The semantic index does not include information about + // `from...import` statements because there are two things it cannot determine while only + // inspecting the content of the current file: + // + // - whether the imported symbol is an attribute or submodule + // - whether the containing file is in a module or a package (needed to correctly resolve + // relative imports) + // + // The first would be solvable by making it a _potentially_ imported modules set. The + // second is not. + // + // Regardless, for now, we sidestep all of that by repeating the submodule-or-attribute + // check here when inferring types for a `from...import` statement. + if let Some(submodule_type) = full_submodule_name + .as_ref() + .and_then(|submodule_name| self.module_type_from_name(submodule_name)) + { + self.add_binding(import_from.into(), definition, |_, _| submodule_type); + return; + } + + self.add_binding(import_from.into(), definition, |_, _| Type::unknown()); + + if !self.is_reachable(import_from) { + return; + } + + let Some(builder) = self + .context + .report_lint(&UNRESOLVED_IMPORT, AnyNodeRef::StmtImportFrom(import_from)) + else { return; }; From 6d7278cd9eec37bb98ccc77b7fc893a7f19a95b2 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Tue, 4 Nov 2025 15:40:21 -0500 Subject: [PATCH 12/14] clean up impl --- .../mdtest/import/nonstandard_conventions.md | 123 +++++++++++++++- .../ty_python_semantic/src/semantic_index.rs | 132 +----------------- .../src/semantic_index/builder.rs | 62 ++++---- .../src/semantic_index/definition.rs | 16 ++- crates/ty_python_semantic/src/types.rs | 28 ++-- .../src/types/infer/builder.rs | 8 +- 6 files changed, 173 insertions(+), 196 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md index fbd3b1c931933e..88f4daa139288f 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md +++ b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md @@ -236,10 +236,11 @@ X: int = 42 ```py import mypackage -reveal_type(mypackage.submodule) # revealed: -# error: "has no member `nested`" +# error: "has no member `submodule`" +reveal_type(mypackage.submodule) # revealed: Unknown +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested) # revealed: Unknown -# error: "has no member `nested`" +# error: "has no member `submodule`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ``` @@ -587,7 +588,8 @@ X: int = 42 ```py import mypackage -reveal_type(mypackage.imported.X) # revealed: int +# error: "no member `imported`" +reveal_type(mypackage.imported.X) # revealed: Unknown ``` ## `from` Import of Non-Submodule (Non-Stub Check) @@ -695,8 +697,8 @@ import mypackage from mypackage import imported reveal_type(imported.X) # revealed: int -# TODO: uhhhh should this work!? -reveal_type(imported.fails.Y) # revealed: int +# error: "has no member `fails`" +reveal_type(imported.fails.Y) # revealed: Unknown # error: "has no member `fails`" reveal_type(mypackage.fails.Y) # revealed: Unknown ``` @@ -837,6 +839,115 @@ def funcmod(x: int) -> int: return x ``` +## Re-export Nameclash Problems In Try-Blocks (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +try: + from .funcmod import funcmod + + funcmod(1) + + def run(): + # TODO: this is a bug in how we analyze try-blocks + # error: [call-non-callable] + funcmod(2) + +finally: + x = 1 +``` + +`mypackage/funcmod.py`: + +```py +def funcmod(x: int) -> int: + return x +``` + +## XXX3 (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +def run1(): + from .funcmod import funcmod + + funcmod(1) + +def run2(): + from .funcmod import funcmod + + funcmod(2) + +def run3(): + # error: [unresolved-reference] + funcmod(3) + +# error: [unresolved-reference] +funcmod(4) +``` + +`mypackage/funcmod.py`: + +```py +def funcmod(x: int) -> int: + return x +``` + +## XXX4 (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +def run1(): + from .funcmod import other + + funcmod.funcmod(1) + +def run2(): + from .funcmod import other + + # TODO: this is just a bug! We only register the first + # import of `funcmod` in the entire file, and not per-scope! + # error: [unresolved-reference] + funcmod.funcmod(2) + +def run3(): + # error: [unresolved-reference] + funcmod.funcmod(3) + +# error: [unresolved-reference] +funcmod.funcmod(4) +``` + +`mypackage/funcmod.py`: + +```py +other: int = 1 + +def funcmod(x: int) -> int: + return x +``` + +## XXX2 (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +funcmod = 0 +from .funcmod import funcmod + +funcmod(1) +``` + +`mypackage/funcmod.py`: + +```py +def funcmod(x: int) -> int: + return x +``` + ## Shadowing Import With Definition `mypackage/__init__.pyi`: diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index 4401ee431b5b08..4d31de2cb97bf9 100644 --- a/crates/ty_python_semantic/src/semantic_index.rs +++ b/crates/ty_python_semantic/src/semantic_index.rs @@ -6,12 +6,12 @@ use ruff_db::parsed::parsed_module; use ruff_index::{IndexSlice, IndexVec}; use ruff_python_ast::NodeIndex; -use ruff_python_ast::name::Name; use ruff_python_parser::semantic_errors::SemanticSyntaxError; use rustc_hash::{FxHashMap, FxHashSet}; use salsa::Update; use salsa::plumbing::AsId; +use crate::Db; use crate::module_name::ModuleName; use crate::node_key::NodeKey; use crate::semantic_index::ast_ids::AstIds; @@ -28,7 +28,6 @@ use crate::semantic_index::scope::{ use crate::semantic_index::symbol::ScopedSymbolId; use crate::semantic_index::use_def::{EnclosingSnapshotKey, ScopedEnclosingSnapshotId, UseDefMap}; use crate::semantic_model::HasTrackedScope; -use crate::{Db, Module, resolve_module}; pub mod ast_ids; mod builder; @@ -84,122 +83,6 @@ pub(crate) fn imported_modules<'db>(db: &'db dyn Db, file: File) -> Arc( - db: &'db dyn Db, - importing_module: Module<'db>, -) -> Box<[ModuleName]> { - // FIXME(Gankra): temporarily disabled - if true { - return Box::default(); - } - let Some(file) = importing_module.file(db) else { - return Box::default(); - }; - if !file.is_package(db) { - return Box::default(); - } - // let is_stub = file.is_package_stub(db); - let importing_module_name = importing_module.name(db); - - let imports = semantic_index(db, file) - .maybe_imported_modules - .iter() - .filter_map(|import| { - let submodule = ModuleName::from_identifier_parts( - db, - file, - import.from_module.as_deref(), - import.level, - ) - .ok()?; - - Some((submodule, import.names.clone())) - }) - .chain( - semantic_index(db, file) - .imported_modules - .iter() - .map(|module| (module.clone(), vec![])), - ) - .collect::>(); - - let created_names = imports - .iter() - .flat_map(|(_, names)| { - names - .iter() - .map(|(name, alias)| alias.as_ref().unwrap_or(name)) - }) - .collect::>(); - - imports - .iter() - .filter_map(|(module, names)| { - let relative = if importing_module_name == module { - None - } else { - Some(module.relative_to(importing_module_name)?) - }; - - // Don't mess with star imports - if names.iter().any(|(name, _)| name.as_str() == "*") { - return None; - } - - if let Some(rel) = relative { - // The module we're importing from is a submodule of ourself! - // In this case the only submodule attribute we're introducing in ourself - // is the direct submodule, but we don't want to do that if another from import - // explicitly shadows it - let direct_submodule = Name::from(rel.components().next()?.to_owned()); - if created_names.contains(&direct_submodule) { - return None; - } - Some(vec![ModuleName::new(direct_submodule.as_str())?]) - } else { - // The module we're importing from is ourselves! - // In this case all imports are presumably of modules, as it "doesn't" - // make sense to write `from . import an_actual_function` - let submodules = names.iter().filter_map(|(name, alias)| { - if alias.is_some() && alias.as_ref() != Some(name) { - return None; - } - let subname = ModuleName::new(name)?; - let mut submodule = importing_module_name.clone(); - submodule.extend(&subname); - resolve_module(db, &submodule)?; - Some(subname) - }); - - Some(submodules.collect()) - } - }) - .flatten() - .collect() -} - /// Returns the use-def map for a specific `scope`. /// /// Using [`use_def_map`] over [`semantic_index`] has the advantage that @@ -341,9 +224,6 @@ pub(crate) struct SemanticIndex<'db> { /// The set of modules that are imported anywhere within this file. imported_modules: Arc>, - /// `from...import` statements within this file that might import a submodule. - maybe_imported_modules: FxHashSet, - /// Flags about the global scope (code usage impacting inference) has_future_annotations: bool, @@ -357,16 +237,6 @@ pub(crate) struct SemanticIndex<'db> { generator_functions: FxHashSet, } -/// A `from...import` that may be an import of a module -/// -/// Later analysis will determine if it is. -#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, get_size2::GetSize)] -pub(crate) struct MaybeModuleImport { - level: u32, - from_module: Option, - names: Vec<(Name, Option)>, -} - impl<'db> SemanticIndex<'db> { /// Returns the place table for a specific scope. /// diff --git a/crates/ty_python_semantic/src/semantic_index/builder.rs b/crates/ty_python_semantic/src/semantic_index/builder.rs index c37f78af138d18..14f662d276365c 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -47,9 +47,7 @@ use crate::semantic_index::symbol::{ScopedSymbolId, Symbol}; use crate::semantic_index::use_def::{ EnclosingSnapshotKey, FlowSnapshot, ScopedEnclosingSnapshotId, UseDefMapBuilder, }; -use crate::semantic_index::{ - ExpressionsScopeMap, MaybeModuleImport, SemanticIndex, VisibleAncestorsIter, -}; +use crate::semantic_index::{ExpressionsScopeMap, SemanticIndex, VisibleAncestorsIter}; use crate::semantic_model::HasTrackedScope; use crate::unpack::{EvaluationMode, Unpack, UnpackKind, UnpackPosition, UnpackValue}; use crate::{Db, Program}; @@ -113,7 +111,6 @@ pub(super) struct SemanticIndexBuilder<'db, 'ast> { definitions_by_node: FxHashMap>, expressions_by_node: FxHashMap>, imported_modules: FxHashSet, - maybe_imported_modules: FxHashSet, seen_submodule_imports: FxHashSet, /// Hashset of all [`FileScopeId`]s that correspond to [generator functions]. /// @@ -153,7 +150,6 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { expressions_by_node: FxHashMap::default(), seen_submodule_imports: FxHashSet::default(), - maybe_imported_modules: FxHashSet::default(), imported_modules: FxHashSet::default(), generator_functions: FxHashSet::default(), @@ -1268,7 +1264,6 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { self.scopes_by_node.shrink_to_fit(); self.generator_functions.shrink_to_fit(); self.enclosing_snapshots.shrink_to_fit(); - self.maybe_imported_modules.shrink_to_fit(); SemanticIndex { place_tables, @@ -1281,7 +1276,6 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { scopes_by_node: self.scopes_by_node, use_def_maps, imported_modules: Arc::new(self.imported_modules), - maybe_imported_modules: self.maybe_imported_modules, has_future_annotations: self.has_future_annotations, enclosing_snapshots: self.enclosing_snapshots, semantic_syntax_errors: self.semantic_syntax_errors.into_inner(), @@ -1455,10 +1449,29 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { self.current_use_def_map_mut() .record_node_reachability(NodeKey::from_node(node)); + // If we see: + // + // * `from .x.y import z` (must be relative!) + // * And we are in an `__init__.py(i)` (hereafter `thispackage`) + // * And this is the first time we've seen `from .x` in this module + // + // We introduce a local definition `x = ` that occurs + // before the `z = ...` declaration the import introduces. This models the fact + // that the *first* time that you import 'thispackage.x' the python runtime creates + // `x` as a variable in the global scope of `thispackage`. + // + // This is not a perfect simulation of actual runtime behaviour for *various* + // reasons but it works well for most practical purposes. In particular it's nice + // that `x` can be freely overwritten, and that we don't assume that an import + // in one function is visible in another function. + // + // TODO: Also support `from thispackage.x.y import z`? + // TODO: `seen_submodule_imports` should be per-scope and not per-file + // (if two functions import `.x`, they both should believe `x` is defined) if node.level == 1 - && let Some(submodule_raw) = &node.module - && let Some(submodule) = ModuleName::new(submodule_raw.as_str()) - && let Some(direct_submodule) = submodule.components().next() + && let Some(submodule) = &node.module + && let Some(parsed_submodule) = ModuleName::new(submodule.as_str()) + && let Some(direct_submodule) = parsed_submodule.components().next() && self.file.is_package(self.db) && !self.seen_submodule_imports.contains(direct_submodule) { @@ -1469,28 +1482,10 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { let symbol = self.add_symbol(direct_submodule_name); self.add_definition( symbol.into(), - ImportFromImplicitDefinitionNodeRef { - node, - direct_submodule_name: submodule_raw, - }, + ImportFromImplicitDefinitionNodeRef { node, submodule }, ); } - self.maybe_imported_modules.insert(MaybeModuleImport { - level: node.level, - from_module: node.module.clone().map(Into::into), - names: node - .names - .iter() - .map(|name| { - ( - name.name.clone().into(), - name.asname.clone().map(Into::into), - ) - }) - .collect(), - }); - let mut found_star = false; for (alias_index, alias) in node.names.iter().enumerate() { if &alias.name == "*" { @@ -1597,9 +1592,16 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> { } let (symbol_name, is_reexported) = if let Some(asname) = &alias.asname { + // It's re-exported if it's `from ... import x as x` (&asname.id, asname.id == alias.name.id) } else { - (&alias.name.id, node.level == 1 && node.module.is_none()) + // It's re-exported if it's `from . import x` in an `__init__.pyi` + ( + &alias.name.id, + node.level == 1 + && node.module.is_none() + && self.file.is_package(self.db), + ) }; // Look for imports `from __future__ import annotations`, ignore `as ...` diff --git a/crates/ty_python_semantic/src/semantic_index/definition.rs b/crates/ty_python_semantic/src/semantic_index/definition.rs index 94525ddb3d76be..2db8d739c064f7 100644 --- a/crates/ty_python_semantic/src/semantic_index/definition.rs +++ b/crates/ty_python_semantic/src/semantic_index/definition.rs @@ -368,7 +368,7 @@ pub(crate) struct ImportFromDefinitionNodeRef<'ast> { #[derive(Copy, Clone, Debug)] pub(crate) struct ImportFromImplicitDefinitionNodeRef<'ast> { pub(crate) node: &'ast ast::StmtImportFrom, - pub(crate) direct_submodule_name: &'ast ast::Identifier, + pub(crate) submodule: &'ast ast::Identifier, } #[derive(Copy, Clone, Debug)] pub(crate) struct AssignmentDefinitionNodeRef<'ast, 'db> { @@ -450,10 +450,10 @@ impl<'db> DefinitionNodeRef<'_, 'db> { }), DefinitionNodeRef::ImportFromImplicit(ImportFromImplicitDefinitionNodeRef { node, - direct_submodule_name, + submodule, }) => DefinitionKind::ImportFromImplicit(ImportFromImplicitDefinitionKind { node: AstNodeRef::new(parsed, node), - direct_submodule_name: direct_submodule_name.as_str().into(), + submodule: submodule.as_str().into(), }), DefinitionNodeRef::ImportStar(star_import) => { let StarImportDefinitionNodeRef { node, symbol_id } = star_import; @@ -582,7 +582,7 @@ impl<'db> DefinitionNodeRef<'_, 'db> { }) => (&node.names[alias_index]).into(), Self::ImportFromImplicit(ImportFromImplicitDefinitionNodeRef { node, - direct_submodule_name: _, + submodule: _, }) => node.into(), // INVARIANT: for an invalid-syntax statement such as `from foo import *, bar, *`, // we only create a `StarImportDefinitionKind` for the *first* `*` alias in the names list. @@ -709,6 +709,7 @@ impl DefinitionKind<'_> { match self { DefinitionKind::Import(import) => import.is_reexported(), DefinitionKind::ImportFrom(import) => import.is_reexported(), + DefinitionKind::ImportFromImplicit(_) => false, _ => true, } } @@ -726,6 +727,7 @@ impl DefinitionKind<'_> { DefinitionKind::Import(_) | DefinitionKind::ImportFrom(_) | DefinitionKind::StarImport(_) + | DefinitionKind::ImportFromImplicit(_) ) } @@ -1019,7 +1021,7 @@ impl ImportFromDefinitionKind { #[derive(Clone, Debug, get_size2::GetSize)] pub struct ImportFromImplicitDefinitionKind { node: AstNodeRef, - direct_submodule_name: Name, + submodule: Name, } impl ImportFromImplicitDefinitionKind { @@ -1027,8 +1029,8 @@ impl ImportFromImplicitDefinitionKind { self.node.node(module) } - pub(crate) fn direct_submodule_name(&self) -> &Name { - &self.direct_submodule_name + pub(crate) fn submodule(&self) -> &Name { + &self.submodule } } diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 3e3c241925cac8..e01852ae89d161 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -39,9 +39,7 @@ use crate::place::{ use crate::semantic_index::definition::{Definition, DefinitionKind}; use crate::semantic_index::place::ScopedPlaceId; use crate::semantic_index::scope::ScopeId; -use crate::semantic_index::{ - imported_modules, imported_relative_submodules_of_stub_package, place_table, semantic_index, -}; +use crate::semantic_index::{imported_modules, place_table, semantic_index}; use crate::suppression::check_suppressions; use crate::types::bound_super::BoundSuperType; use crate::types::call::{Binding, Bindings, CallArguments, CallableBinding}; @@ -10984,29 +10982,23 @@ impl<'db> ModuleLiteralType<'db> { /// /// # Rules /// - /// We have two rules for whether a submodule attribute is defined: + /// Because of the excessive power and danger of this method, we currently have only one rule: /// - /// * If the importing file include `import x.y` then `x.y` is defined in the importing file. - /// This is an easy rule to justify because `import` can only ever import a module, and so + /// * If the importing file includes `import x.y` then `x.y` is defined in the importing file. + /// This is an easy rule to justify because `import` can only ever import a module, and the + /// only reason to do it is to explicitly introduce those submodules and attributes, so it /// *should* shadow any non-submodule of the same name. /// - /// * If the module is an `__init__.pyi` for `mypackage`, and it contains a `from...import` - /// that normalizes to `from mypackage import submodule`, then `mypackage.submodule` is - /// defined in all files. This supports the `from . import submodule` idiom. Critically, - /// we do *not* allow `from mypackage.nested import submodule` to affect `mypackage`. - /// The idea here is that `from mypackage import submodule` *from mypackage itself* can - /// only ever reasonably be an import of a submodule. It doesn't make any sense to import - /// a function or class from yourself! (You *can* do it but... why? Don't? Please?) + /// `import x.y from z` instances are currently ignored because the `x.y` part may not be a + /// side-effect the user actually cares about, and the `z` component may not be a submodule. + /// + /// We instead prefer handling most other import effects as definitions in the scope of + /// the current file (i.e. [`crate::semantic_index::definition::ImportFromDefinitionNodeRef`]). fn available_submodule_attributes(&self, db: &'db dyn Db) -> impl Iterator { self.importing_file(db) .into_iter() .flat_map(|file| imported_modules(db, file)) .filter_map(|submodule_name| submodule_name.relative_to(self.module(db).name(db))) - .chain( - imported_relative_submodules_of_stub_package(db, self.module(db)) - .iter() - .cloned(), - ) .filter_map(|relative_submodule| relative_submodule.components().next().map(Name::from)) } diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 6a04fa8f643f97..a990ecb5ad268d 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -1204,7 +1204,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { DefinitionKind::ImportFromImplicit(import_from) => { self.infer_import_from_implicit_definition( import_from.import(self.module()), - import_from.direct_submodule_name(), + import_from.submodule(), definition, ); } @@ -5486,13 +5486,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { fn infer_import_from_implicit_definition( &mut self, import_from: &ast::StmtImportFrom, - direct_submodule_name: &Name, + submodule: &Name, definition: Definition<'db>, ) { - let name = direct_submodule_name + let name = submodule .split_once('.') .map(|(first, _)| first) - .unwrap_or(direct_submodule_name.as_str()); + .unwrap_or(submodule.as_str()); let Ok(module_name) = ModuleName::from_identifier_parts(self.db(), self.file(), None, 1) else { self.add_binding(import_from.into(), definition, |_, _| Type::unknown()); From fc9db79745744b29611437878ccae34f993e8045 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Wed, 5 Nov 2025 09:42:54 -0500 Subject: [PATCH 13/14] clean up docs --- .../mdtest/import/nonstandard_conventions.md | 154 +++++++++++------- .../src/types/infer/builder.rs | 60 ++++--- 2 files changed, 132 insertions(+), 82 deletions(-) diff --git a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md index 88f4daa139288f..28b03e7ee6e106 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md +++ b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md @@ -1,39 +1,35 @@ # Nonstandard Import Conventions This document covers ty-specific extensions to the -[standard import conventions](https://typing.python.org/en/latest/spec/distributing.html#import-conventions). - -It's a common idiom for a package's `__init__.py(i)` to include several imports like -`from . import mysubmodule`, with the intent that the `mypackage.mysubmodule` attribute should work -for anyone who only imports `mypackage`. - -In the context of a `.py` we handle this well through our general attempts to faithfully implement -import side-effects. However for `.pyi` files we are expected to apply -[a more strict set of rules](https://typing.python.org/en/latest/spec/distributing.html#import-conventions) -to encourage intentional API design. Although `.pyi` files are explicitly designed to work with -typecheckers, which ostensibly should all enforce these strict rules, every typechecker has its own -defacto "extensions" to them and so a few idioms like `from . import mysubmodule` have found their -way into `.pyi` files too. - -Thus for the sake of compatibility, we need to define our own "extensions". Any extensions we define -here have several competing concerns: - -- Extensions should ideally be kept narrow to continue to encourage explicit API design -- Extensions should be easy to explain, document, and understand -- Extensions should ideally still be a subset of runtime behaviour (if it works in a stub, it works - at runtime) -- Extensions should ideally not make `.pyi` files more permissive than `.py` files (if it works in a - stub, it works in an impl) - -To that end we define the following extension: - -> If an `__init__.pyi` for `mypackage` contains a `from...import` targetting a direct submodule of -> `mypackage`, then that submodule should be available as an attribute of `mypackage`. +[standard import conventions](https://typing.python.org/en/latest/spec/distributing.html#import-conventions), +and other intentional deviations from actual python semantics. + +This file currently covers the following details: + +- **froms are locals**: a `from..import` can only define locals, it does not have global + side-effects. Specifically any submodule attribute `a` that's implicitly introduced by either + `from .a import b` or `from . import a as b` (in an `__init__.py(i)`) is a local and not a + global. If you do such an import at the top of a file you won't notice this. However if you do + such an import in a function, that means it will only be function-scoped (so you'll need to do + it in every function that wants to access it, making your code less sensitive to execution + order). + +- **first from first serve**: only the *first* `from..import` in an `__init__.py(i)` that imports a + particular direct submodule of the current package introduces that submodule as a local. + Subsequent imports of the submodule will not introduce that local. This reflects the fact that + in actual python only the first import of a submodule (in the entire execution of the program) + introduces it as an attribute of the package. By "first" we mean "the first time in this scope + (or any parent scope)". This pairs well with the fact that we are specifically introducing a + local (as long as you don't accidentally shadow or overwrite the local). + +- **dot re-exports**: `from . import a` in an `__init__.pyi` is considered a re-export of `a` + (equivalent to `from . import a as a`). This is required to properly handle many stubs in the + wild. Currently it must be *exactly* `from . import ...`. ## Relative `from` Import of Direct Submodule in `__init__` -The `from . import submodule` idiom in an `__init__.pyi` is fairly explicit and we should definitely -support it. +The `from . import submodule` idiom in an `__init__.pyi` should definitely be considered an explicit +re-export. `mypackage/__init__.pyi`: @@ -95,8 +91,9 @@ reveal_type(mypackage.fails.Y) # revealed: Unknown ## Absolute `from` Import of Direct Submodule in `__init__` -If an absolute `from...import` happens to import a submodule, it works just as well as a relative -one. +If an absolute `from...import` happens to import a submodule (i.e. it's equivalent to +`from . import y`) we do not treat it as a re-export. We could, but we don't. (This is an arbitrary +decision and can be changed!) `mypackage/__init__.pyi`: @@ -121,7 +118,7 @@ Y: int = 47 ```py import mypackage -# TODO: this could work and would be nice to have +# TODO: this could work and would be nice to have? # error: "has no member `imported`" reveal_type(mypackage.imported.X) # revealed: Unknown # error: "has no member `fails`" @@ -161,7 +158,7 @@ reveal_type(mypackage.fails.Y) # revealed: Unknown ## Import of Direct Submodule in `__init__` An `import` that happens to import a submodule does not expose the submodule as an attribute. (This -is an arbitrary decision and can be changed easily!) +is an arbitrary decision and can be changed!) `mypackage/__init__.pyi`: @@ -180,7 +177,7 @@ X: int = 42 ```py import mypackage -# TODO: this could work and would be nice to have +# TODO: this could work and would be nice to have? # error: "has no member `imported`" reveal_type(mypackage.imported.X) # revealed: Unknown ``` @@ -211,8 +208,8 @@ reveal_type(mypackage.imported.X) # revealed: Unknown ## Relative `from` Import of Nested Submodule in `__init__` -`from .submodule import nested` in an `__init__.pyi` is currently not supported as a way to expose -`mypackage.submodule` or `mypackage.submodule.nested` but it could be. +`from .submodule import nested` in an `__init__.pyi` does not re-export `mypackage.submodule`, +`mypackage.submodule.nested`, or `nested`. `mypackage/__init__.pyi`: @@ -242,10 +239,16 @@ reveal_type(mypackage.submodule) # revealed: Unknown reveal_type(mypackage.submodule.nested) # revealed: Unknown # error: "has no member `submodule`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +# error: "has no member `nested`" +reveal_type(mypackage.nested) # revealed: Unknown +# error: "has no member `nested`" +reveal_type(mypackage.nested.X) # revealed: Unknown ``` ## Relative `from` Import of Nested Submodule in `__init__` (Non-Stub Check) +`from .submodule import nested` in an `__init__.py` exposes `mypackage.submodule` and `nested`. + `mypackage/__init__.py`: ```py @@ -274,12 +277,14 @@ reveal_type(mypackage.submodule) # revealed: reveal_type(mypackage.submodule.nested) # revealed: Unknown # error: "has no member `nested`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +reveal_type(mypackage.nested) # revealed: +reveal_type(mypackage.nested.X) # revealed: int ``` ## Absolute `from` Import of Nested Submodule in `__init__` -`from mypackage.submodule import nested` in an `__init__.pyi` is currently not supported as a way to -expose `mypackage.submodule` or `mypackage.submodule.nested` but it could be. +`from mypackage.submodule import nested` in an `__init__.pyi` does not re-export +`mypackage.submodule`, `mypackage.submodule.nested`, or `nested`. `mypackage/__init__.pyi`: @@ -310,10 +315,16 @@ reveal_type(mypackage.submodule) # revealed: Unknown reveal_type(mypackage.submodule.nested) # revealed: Unknown # error: "has no member `submodule`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +# error: "has no member `nested`" +reveal_type(mypackage.nested) # revealed: Unknown +# error: "has no member `nested`" +reveal_type(mypackage.nested.X) # revealed: Unknown ``` ## Absolute `from` Import of Nested Submodule in `__init__` (Non-Stub Check) +`from mypackage.submodule import nested` in an `__init__.py` only creates `nested`. + `mypackage/__init__.py`: ```py @@ -336,19 +347,21 @@ X: int = 42 ```py import mypackage +# TODO: this would be nice to support # error: "has no member `submodule`" reveal_type(mypackage.submodule) # revealed: Unknown -# TODO: this would be nice to support # error: "has no member `submodule`" reveal_type(mypackage.submodule.nested) # revealed: Unknown # error: "has no member `submodule`" reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +reveal_type(mypackage.nested) # revealed: +reveal_type(mypackage.nested.X) # revealed: int ``` ## Import of Nested Submodule in `__init__` -`import mypackage.submodule.nested` in an `__init__.pyi` is currently not supported as a way to -expose `mypackage.submodule` or `mypackage.submodule.nested` but it could be. +`import mypackage.submodule.nested` in an `__init__.pyi` does not re-export `mypackage.submodule` or +`mypackage.submodule.nested`. `mypackage/__init__.pyi`: @@ -382,6 +395,9 @@ reveal_type(mypackage.submodule.nested.X) # revealed: Unknown ## Import of Nested Submodule in `__init__` (Non-Stub Check) +`import mypackage.submodule.nested` in an `__init__.py` does not define `mypackage.submodule` or +`mypackage.submodule.nested` outside the package. + `mypackage/__init__.py`: ```py @@ -404,6 +420,7 @@ X: int = 42 ```py import mypackage +# TODO: this would be nice to support # error: "has no member `submodule`" reveal_type(mypackage.submodule) # revealed: Unknown # error: "has no member `submodule`" @@ -534,7 +551,7 @@ X: int = 42 ```py from mypackage import * -# TODO: this would be nice to support (available_submodule_attributes isn't visible to `*` imports) +# TODO: this would be nice to support # error: "`imported` used when not defined" reveal_type(imported.X) # revealed: Unknown reveal_type(Z) # revealed: int @@ -567,9 +584,8 @@ reveal_type(Z) # revealed: int ## `from` Import of Non-Submodule -A from import that terminates in a non-submodule should not expose the intermediate submodules as -attributes. This is an arbitrary decision but on balance probably safe and correct, as otherwise it -would be hard for a stub author to be intentional about the submodules being exposed as attributes. +A `from` import that imports a non-submodule isn't currently a special case here (various +proposed/tested approaches did treat this specially). `mypackage/__init__.pyi`: @@ -748,9 +764,8 @@ Can easily result in the typechecker getting "confused" and thinking imports of top-level package are referring to the subpackage and not the function/class. This issue can be found with the `lobpcg` function in `scipy.sparse.linalg`. -This kind of failure mode is why the rule is restricted to *direct* submodule imports, as anything -more powerful than that in the current implementation strategy quickly gets the functions and -submodules mixed up. +We avoid this by ~desugarring the imports into two definitions, with the RHS occuring second, so +that it can freely overwrite the LHS. `mypackage/__init__.pyi`: @@ -821,6 +836,8 @@ x = funcmod(1) ## Re-export Nameclash Problems In Functions (Non-Stub Check) +`from` imports in an `__init__.py` at file scope should be visible to functions defined in the file: + `mypackage/__init__.py`: ```py @@ -841,6 +858,9 @@ def funcmod(x: int) -> int: ## Re-export Nameclash Problems In Try-Blocks (Non-Stub Check) +`from` imports in an `__init__.py` at file scope in a `try` block should be visible to functions +defined in the `try` block (regression test for a bug): + `mypackage/__init__.py`: ```py @@ -865,7 +885,9 @@ def funcmod(x: int) -> int: return x ``` -## XXX3 (Non-Stub Check) +## RHS `from` Imports In Functions (Non-Stub Check) + +If a `from` import occurs in a function, the RHS symbols should only be visible in that function. `mypackage/__init__.py`: @@ -895,7 +917,11 @@ def funcmod(x: int) -> int: return x ``` -## XXX4 (Non-Stub Check) +## LHS `from` Imports In Functions (Non-Stub Check) + +If a `from` import occurs in a function, LHS symbols should only be visible in that function. This +very blatantly is not runtime-accurate, but exists to try to force you to write "obviously +deterministically correct" imports instead of relying on execution order. `mypackage/__init__.py`: @@ -930,7 +956,11 @@ def funcmod(x: int) -> int: return x ``` -## XXX2 (Non-Stub Check) +## LHS `from` Imports Overwrite Locals (Non-Stub Check) + +The LHS of a `from..import` introduces a local symbol that overwrites any local with the same name. +This reflects actual runtime behaviour, although we're kinda assuming it hasn't been imported +already. `mypackage/__init__.py`: @@ -948,7 +978,10 @@ def funcmod(x: int) -> int: return x ``` -## Shadowing Import With Definition +## LHS `from` Imports Overwritten By Local Function + +The LHS of a `from..import` introduces a local symbol that can be overwritten by defining a function +(or class) with the same name. `mypackage/__init__.pyi`: @@ -972,7 +1005,7 @@ from mypackage import funcmod x = funcmod(1) ``` -## Shadowing Import With Definition (Non-Stub Check) +## LHS `from` Imports Overwritten By Local Function (Non-Stub Check) `mypackage/__init__.py`: @@ -998,7 +1031,9 @@ from mypackage import funcmod x = funcmod(1) ``` -## Shadowing Import With Assignment +## LHS `from` Imports Overwritten By Local Assignment + +The LHS of a `from..import` introduces a local symbol that can be overwritten by assigning to it. `mypackage/__init__.pyi`: @@ -1022,7 +1057,7 @@ from mypackage import funcmod x = funcmod(1) ``` -## Shadowing Import With Assignment (Non-Stub Check) +## LHS `from` Imports Overwritten By Local Assignment (Non-Stub Check) `mypackage/__init__.py`: @@ -1047,7 +1082,10 @@ from mypackage import funcmod x = funcmod(1) ``` -## Shadowing Import Not Clobbered By Second Import +## LHS `from` Imports Only Apply The First Time + +The LHS of a `from..import` of a submodule introduces a local symbol only the first time it +introduces a direct submodule. The second time does nothing. `mypackage/__init__.pyi`: @@ -1071,7 +1109,7 @@ from mypackage import funcmod x = funcmod(1) ``` -## Shadowing Import Not Clobbered By Second Import (Non-Stub Check) +## LHS `from` Imports Only Apply The First Time (Non-Stub Check) `mypackage/__init__.py`: diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index a990ecb5ad268d..499b864e4b507b 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -5483,16 +5483,32 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } } + /// Infer the implicit local definition `x = ` that + /// `from .x.y import z` can introduce in an `__init__.py(i)`. + /// + /// For the definition `z`, see [`infer_import_from_definition`]. fn infer_import_from_implicit_definition( &mut self, import_from: &ast::StmtImportFrom, submodule: &Name, definition: Definition<'db>, ) { - let name = submodule - .split_once('.') - .map(|(first, _)| first) - .unwrap_or(submodule.as_str()); + // Although the *actual* runtime semantic of this kind of statement is to + // introduce a variable in the global scope of this module, we want to + // encourage users to write code that doesn't have dependence on execution-order. + // + // By introducing it as a local variable in the scope the import occurs in, + // we effectively require the developer to either do the import at the start of + // the file where it belongs, or to repeat the import in every function that + // wants to use it, which "definitely" works. + // + // (It doesn't actually "definitely" work because only the first import of `thispackage.x` + // will ever set `x`, and any subsequent overwrites of it will permanently clobber it. + // Also, a local variable `x` in a function should always shadow the submodule because + // the submodule is defined at file-scope. However, both of these issues are much more + // narrow, so this approach seems to work well in practice!) + + // Get this package's module by resolving `.` let Ok(module_name) = ModuleName::from_identifier_parts(self.db(), self.file(), None, 1) else { self.add_binding(import_from.into(), definition, |_, _| Type::unknown()); @@ -5504,39 +5520,35 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { return; }; - // Evaluate whether `X.Y` would constitute a valid submodule name, - // given a `from X import Y` statement. If it is valid, this will be `Some()`; - // else, it will be `None`. - + // Now construct the submodule `.x` + assert!( + !submodule.is_empty(), + "ImportFromImplicitDefinitionKind constructed with empty module" + ); + let name = submodule + .split_once('.') + .map(|(first, _)| first) + .unwrap_or(submodule.as_str()); let full_submodule_name = ModuleName::new(name).map(|final_part| { let mut ret = module_name.clone(); ret.extend(&final_part); ret }); - - // If the module doesn't bind the symbol, check if it's a submodule. This won't get - // handled by the `Type::member` call because it relies on the semantic index's - // `imported_modules` set. The semantic index does not include information about - // `from...import` statements because there are two things it cannot determine while only - // inspecting the content of the current file: - // - // - whether the imported symbol is an attribute or submodule - // - whether the containing file is in a module or a package (needed to correctly resolve - // relative imports) - // - // The first would be solvable by making it a _potentially_ imported modules set. The - // second is not. - // - // Regardless, for now, we sidestep all of that by repeating the submodule-or-attribute - // check here when inferring types for a `from...import` statement. + // And try to import it if let Some(submodule_type) = full_submodule_name .as_ref() .and_then(|submodule_name| self.module_type_from_name(submodule_name)) { + // Success, introduce a binding! + // + // We explicitly don't introduce a *definition* because it's actual ok + // (and fairly common) to overwrite this import with a function or class + // and we don't want it to be a type error to do so. self.add_binding(import_from.into(), definition, |_, _| submodule_type); return; } + // That didn't work, try to produce diagnostics self.add_binding(import_from.into(), definition, |_, _| Type::unknown()); if !self.is_reachable(import_from) { From 8ab19c1287756aac2e63234265ed57ad7908e7fd Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Wed, 5 Nov 2025 09:52:00 -0500 Subject: [PATCH 14/14] fixup --- crates/ty_python_semantic/src/types/infer/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index 499b864e4b507b..8ff9c5270ac1fd 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -5486,7 +5486,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { /// Infer the implicit local definition `x = ` that /// `from .x.y import z` can introduce in an `__init__.py(i)`. /// - /// For the definition `z`, see [`infer_import_from_definition`]. + /// For the definition `z`, see [`TypeInferenceBuilder::infer_import_from_definition`]. fn infer_import_from_implicit_definition( &mut self, import_from: &ast::StmtImportFrom,