diff --git a/crates/ruff_db/src/files.rs b/crates/ruff_db/src/files.rs index 754b65642a..4d57162c7c 100644 --- a/crates/ruff_db/src/files.rs +++ b/crates/ruff_db/src/files.rs @@ -470,6 +470,11 @@ impl File { self.source_type(db).is_stub() } + /// Returns `true` if the file is an `__init__.pyi` + pub fn is_package_stub(self, db: &dyn Db) -> bool { + self.path(db).as_str().ends_with("__init__.pyi") + } + 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 new file mode 100644 index 0000000000..848eaae387 --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md @@ -0,0 +1,824 @@ +# 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`. + +## 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. + +`mypackage/__init__.pyi`: + +```pyi +from . import imported +``` + +`mypackage/imported.pyi`: + +```pyi +X: int = 42 +``` + +`mypackage/fails.pyi`: + +```pyi +Y: int = 47 +``` + +`main.py`: + +```py +import mypackage + +reveal_type(mypackage.imported.X) # revealed: int +# error: "has no member `fails`" +reveal_type(mypackage.fails.Y) # revealed: Unknown +``` + +## Relative `from` Import of Direct Submodule in `__init__` (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from . import imported +``` + +`mypackage/imported.py`: + +```py +X: int = 42 +``` + +`mypackage/fails.py`: + +```py +Y: int = 47 +``` + +`main.py`: + +```py +import mypackage + +reveal_type(mypackage.imported.X) # revealed: int +# error: "has no member `fails`" +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. + +`mypackage/__init__.pyi`: + +```pyi +from mypackage import imported +``` + +`mypackage/imported.pyi`: + +```pyi +X: int = 42 +``` + +`mypackage/fails.pyi`: + +```pyi +Y: int = 47 +``` + +`main.py`: + +```py +import mypackage + +reveal_type(mypackage.imported.X) # revealed: int +# error: "has no member `fails`" +reveal_type(mypackage.fails.Y) # revealed: Unknown +``` + +## Absolute `from` Import of Direct Submodule in `__init__` (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from mypackage import imported +``` + +`mypackage/imported.py`: + +```py +X: int = 42 +``` + +`mypackage/fails.py`: + +```py +Y: int = 47 +``` + +`main.py`: + +```py +import mypackage + +reveal_type(mypackage.imported.X) # revealed: int +# error: "has no member `fails`" +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!) + +`mypackage/__init__.pyi`: + +```pyi +import mypackage.imported +``` + +`mypackage/imported.pyi`: + +```pyi +X: int = 42 +``` + +`main.py`: + +```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 +``` + +## Import of Direct Submodule in `__init__` (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +import mypackage.imported +``` + +`mypackage/imported.py`: + +```py +X: int = 42 +``` + +`main.py`: + +```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 +``` + +## 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. + +`mypackage/__init__.pyi`: + +```pyi +from .submodule import nested +``` + +`mypackage/submodule/__init__.pyi`: + +```pyi +``` + +`mypackage/submodule/nested.pyi`: + +```pyi +X: int = 42 +``` + +`main.py`: + +```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.nested) # revealed: Unknown +# error: "has no member `submodule`" +reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +``` + +## Relative `from` Import of Nested Submodule in `__init__` (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from .submodule import nested +``` + +`mypackage/submodule/__init__.py`: + +```py +``` + +`mypackage/submodule/nested.py`: + +```py +X: int = 42 +``` + +`main.py`: + +```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.nested) # revealed: Unknown +# error: "has no member `submodule`" +reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +``` + +## 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. + +`mypackage/__init__.pyi`: + +```pyi +from mypackage.submodule import nested +``` + +`mypackage/submodule/__init__.pyi`: + +```pyi +``` + +`mypackage/submodule/nested.pyi`: + +```pyi +X: int = 42 +``` + +`main.py`: + +```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.nested) # revealed: Unknown +# error: "has no member `submodule`" +reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +``` + +## Absolute `from` Import of Nested Submodule in `__init__` (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from mypackage.submodule import nested +``` + +`mypackage/submodule/__init__.py`: + +```py +``` + +`mypackage/submodule/nested.py`: + +```py +X: int = 42 +``` + +`main.py`: + +```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.nested) # revealed: Unknown +# error: "has no member `submodule`" +reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +``` + +## 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. + +`mypackage/__init__.pyi`: + +```pyi +import mypackage.submodule.nested +``` + +`mypackage/submodule/__init__.pyi`: + +```pyi +``` + +`mypackage/submodule/nested.pyi`: + +```pyi +X: int = 42 +``` + +`main.py`: + +```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.nested) # revealed: Unknown +# error: "has no member `submodule`" +reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +``` + +## Import of Nested Submodule in `__init__` (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +import mypackage.submodule.nested +``` + +`mypackage/submodule/__init__.py`: + +```py +``` + +`mypackage/submodule/nested.py`: + +```py +X: int = 42 +``` + +`main.py`: + +```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.nested) # revealed: Unknown +# error: "has no member `submodule`" +reveal_type(mypackage.submodule.nested.X) # revealed: Unknown +``` + +## Relative `from` Import of Direct Submodule in `__init__`, Mismatched Alias + +Renaming the submodule to something else disables the `__init__.pyi` idiom. + +`mypackage/__init__.pyi`: + +```pyi +from . import imported as imported_m +``` + +`mypackage/imported.pyi`: + +```pyi +X: int = 42 +``` + +`main.py`: + +```py +import mypackage + +# error: "has no member `imported`" +reveal_type(mypackage.imported.X) # revealed: Unknown +# error: "has no member `imported_m`" +reveal_type(mypackage.imported_m.X) # revealed: Unknown +``` + +## Relative `from` Import of Direct Submodule in `__init__`, Mismatched Alias (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from . import imported as imported_m +``` + +`mypackage/imported.py`: + +```py +X: int = 42 +``` + +`main.py`: + +```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_m.X) # revealed: int +``` + +## Relative `from` Import of Direct Submodule in `__init__`, Matched Alias + +The `__init__.pyi` idiom should definitely always work if the submodule is renamed to itself, as +this is the re-export idiom. + +`mypackage/__init__.pyi`: + +```pyi +from . import imported as imported +``` + +`mypackage/imported.pyi`: + +```pyi +X: int = 42 +``` + +`main.py`: + +```py +import mypackage + +reveal_type(mypackage.imported.X) # revealed: int +``` + +## Relative `from` Import of Direct Submodule in `__init__`, Matched Alias (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from . import imported as imported +``` + +`mypackage/imported.py`: + +```py +X: int = 42 +``` + +`main.py`: + +```py +import mypackage + +reveal_type(mypackage.imported.X) # revealed: int +``` + +## Star Import Unaffected + +Even if the `__init__` idiom is in effect, star imports do not pick it up. (This is an arbitrary +decision that mostly fell out of the implementation details and can be changed!) + +`mypackage/__init__.pyi`: + +```pyi +from . import imported +Z: int = 17 +``` + +`mypackage/imported.pyi`: + +```pyi +X: int = 42 +``` + +`main.py`: + +```py +from mypackage import * + +# TODO: this would be nice to support (available_submodule_attributes isn't visible to `*` imports) +# error: "`imported` used when not defined" +reveal_type(imported.X) # revealed: Unknown +reveal_type(Z) # revealed: int +``` + +## Star Import Unaffected (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from . import imported + +Z: int = 17 +``` + +`mypackage/imported.py`: + +```py +X: int = 42 +``` + +`main.py`: + +```py +from mypackage import * + +reveal_type(imported.X) # revealed: int +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. + +`mypackage/__init__.pyi`: + +```pyi +from .imported import X +``` + +`mypackage/imported.pyi`: + +```pyi +X: int = 42 +``` + +`main.py`: + +```py +import mypackage + +# error: "has no member `imported`" +reveal_type(mypackage.imported.X) # revealed: Unknown +``` + +## `from` Import of Non-Submodule (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from .imported import X +``` + +`mypackage/imported.py`: + +```py +X: int = 42 +``` + +`main.py`: + +```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 +``` + +## `from` Import of Other Package's Submodule + +`from mypackage import submodule` from outside the package is not modeled as a side-effect on +`mypackage`, even in the importing file (this could be changed!). + +`mypackage/__init__.pyi`: + +```pyi +``` + +`mypackage/imported.pyi`: + +```pyi +X: int = 42 +``` + +`main.py`: + +```py +import mypackage +from mypackage import imported + +# TODO: this would be nice to support, but it's dangerous with available_submodule_attributes +reveal_type(imported.X) # revealed: int +# error: "has no member `imported`" +reveal_type(mypackage.imported.X) # revealed: Unknown +``` + +## `from` Import of Other Package's Submodule (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +``` + +`mypackage/imported.py`: + +```py +X: int = 42 +``` + +`main.py`: + +```py +import mypackage +from mypackage import imported + +# TODO: this would be nice to support, as it works at runtime +reveal_type(imported.X) # revealed: int +# error: "has no member `imported`" +reveal_type(mypackage.imported.X) # revealed: Unknown +``` + +## `from` Import of Sibling Module + +`from . import submodule` from a sibling module is not modeled as a side-effect on `mypackage` or a +re-export from `submodule`. + +`mypackage/__init__.pyi`: + +```pyi +``` + +`mypackage/imported.pyi`: + +```pyi +from . import fails +X: int = 42 +``` + +`mypackage/fails.pyi`: + +```pyi +Y: int = 47 +``` + +`main.py`: + +```py +import mypackage +from mypackage import imported + +reveal_type(imported.X) # 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 +``` + +## `from` Import of Sibling Module (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +``` + +`mypackage/imported.py`: + +```py +from . import fails + +X: int = 42 +``` + +`mypackage/fails.py`: + +```py +Y: int = 47 +``` + +`main.py`: + +```py +import mypackage +from mypackage import imported + +reveal_type(imported.X) # revealed: int +reveal_type(imported.fails.Y) # revealed: int +# error: "has no member `fails`" +reveal_type(mypackage.fails.Y) # revealed: Unknown +``` + +## Fractal Re-export Nameclash Problems + +This precise configuration of: + +- a subpackage that defines a submodule with its own name +- that in turn defines a function/class with its own name +- and re-exporting that name through every layer using `from` imports and `__all__` + +Can easily result in the typechecker getting "confused" and thinking imports of the name from the +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. + +`mypackage/__init__.pyi`: + +```pyi +from .funcmod import funcmod + +__all__ = ["funcmod"] +``` + +`mypackage/funcmod/__init__.pyi`: + +```pyi +from .funcmod import funcmod + +__all__ = ["funcmod"] +``` + +`mypackage/funcmod/funcmod.pyi`: + +```pyi +__all__ = ["funcmod"] + +def funcmod(x: int) -> int: ... +``` + +`main.py`: + +```py +from mypackage import funcmod + +x = funcmod(1) +``` + +## Fractal Re-export Nameclash Problems (Non-Stub Check) + +`mypackage/__init__.py`: + +```py +from .funcmod import funcmod + +__all__ = ["funcmod"] +``` + +`mypackage/funcmod/__init__.py`: + +```py +from .funcmod import funcmod + +__all__ = ["funcmod"] +``` + +`mypackage/funcmod/funcmod.py`: + +```py +__all__ = ["funcmod"] + +def funcmod(x: int) -> int: + return x +``` + +`main.py`: + +```py +from mypackage import funcmod + +x = funcmod(1) +``` diff --git a/crates/ty_python_semantic/src/semantic_index.rs b/crates/ty_python_semantic/src/semantic_index.rs index 558243f59c..a654873db3 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,6 +28,7 @@ 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; @@ -75,20 +76,73 @@ pub(crate) fn place_table<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> Arc(db: &'db dyn Db, file: File) -> Arc> { semantic_index(db, file).imported_modules.clone() } +/// Returns the set of relative submodules that are explicitly imported anywhere in +/// `importing_module`. +/// +/// This set only considers `from...import` statements (but it could also include `import`). +/// It also only returns a non-empty result for `__init__.pyi` files. +/// See [`ModuleLiteralType::available_submodule_attributes`] for discussion +/// of why this analysis is intentionally limited. +/// +/// This function specifically implements the rule that if an `__init__.pyi` file +/// contains a `from...import` that imports a direct submodule of the package, +/// that submodule should be available as an attribute of the package. +/// +/// While we endeavour to accurately model import side-effects for `.py` files, we intentionally +/// limit them for `.pyi` files to encourage more intentional API design. The standard escape +/// hatches for this are the `import x as x` idiom or listing them in `__all__`, but in practice +/// some other idioms are popular. +/// +/// In particular, many packages have their `__init__` include lines like +/// `from . import subpackage`, with the intent that `mypackage.subpackage` should be +/// available for anyone who only does `import mypackage`. +#[salsa::tracked(returns(deref), heap_size=ruff_memory_usage::heap_size)] +pub(crate) fn imported_relative_submodules_of_stub_package<'db>( + db: &'db dyn Db, + importing_module: Module<'db>, +) -> Box<[ModuleName]> { + let Some(file) = importing_module.file(db) else { + return Box::default(); + }; + if !file.is_package_stub(db) { + return Box::default(); + } + semantic_index(db, file) + .maybe_imported_modules + .iter() + .filter_map(|import| { + let mut submodule = ModuleName::from_identifier_parts( + db, + file, + import.from_module.as_deref(), + 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; + } + 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() +} + /// Returns the use-def map for a specific `scope`. /// /// Using [`use_def_map`] over [`semantic_index`] has the advantage that @@ -230,6 +284,9 @@ 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, @@ -243,6 +300,16 @@ 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, + submodule: Name, +} + 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 8107f9c122..5645fed7d4 100644 --- a/crates/ty_python_semantic/src/semantic_index/builder.rs +++ b/crates/ty_python_semantic/src/semantic_index/builder.rs @@ -47,7 +47,9 @@ use crate::semantic_index::symbol::{ScopedSymbolId, Symbol}; use crate::semantic_index::use_def::{ EnclosingSnapshotKey, FlowSnapshot, ScopedEnclosingSnapshotId, UseDefMapBuilder, }; -use crate::semantic_index::{ExpressionsScopeMap, SemanticIndex, VisibleAncestorsIter}; +use crate::semantic_index::{ + ExpressionsScopeMap, MaybeModuleImport, SemanticIndex, VisibleAncestorsIter, +}; use crate::semantic_model::HasTrackedScope; use crate::unpack::{EvaluationMode, Unpack, UnpackKind, UnpackPosition, UnpackValue}; use crate::{Db, Program}; @@ -111,6 +113,7 @@ pub(super) struct SemanticIndexBuilder<'db, 'ast> { definitions_by_node: FxHashMap>, expressions_by_node: FxHashMap>, imported_modules: FxHashSet, + maybe_imported_modules: FxHashSet, /// Hashset of all [`FileScopeId`]s that correspond to [generator functions]. /// /// [generator functions]: https://docs.python.org/3/glossary.html#term-generator @@ -148,6 +151,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> { definitions_by_node: FxHashMap::default(), expressions_by_node: FxHashMap::default(), + maybe_imported_modules: FxHashSet::default(), imported_modules: FxHashSet::default(), generator_functions: FxHashSet::default(), @@ -1262,6 +1266,7 @@ 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, @@ -1274,6 +1279,7 @@ 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(), @@ -1558,6 +1564,15 @@ 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 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__` diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 6b48499e9b..be3816ac12 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -39,7 +39,9 @@ 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, place_table, semantic_index}; +use crate::semantic_index::{ + imported_modules, imported_relative_submodules_of_stub_package, place_table, semantic_index, +}; use crate::suppression::check_suppressions; use crate::types::bound_super::BoundSuperType; use crate::types::call::{Binding, Bindings, CallArguments, CallableBinding}; @@ -10830,11 +10832,68 @@ impl<'db> ModuleLiteralType<'db> { self._importing_file(db) } + /// Get the submodule attributes we believe to be defined on this module. + /// + /// Note that `ModuleLiteralType` is per-importing-file, so this analysis + /// includes "imports the importing file has performed". + /// + /// + /// # Danger! Powerful Hammer! + /// + /// These results immediately make the attribute always defined in the importing file, + /// shadowing any other attribute in the module with the same name, even if the + /// non-submodule-attribute is in fact always the one defined in practice. + /// + /// Intuitively this means `available_submodule_attributes` "win all tie-breaks", + /// with the idea that if we're ever confused about complicated code then usually + /// the import is the thing people want in scope. + /// + /// However this "always defined, always shadows" rule if applied too aggressively + /// creates VERY confusing conclusions that break perfectly reasonable code. + /// + /// For instance, consider a package which has a `myfunc` submodule which defines a + /// `myfunc` function (a common idiom). If the package "re-exports" this function + /// (`from .myfunc import myfunc`), then at runtime in python + /// `from mypackage import myfunc` should import the function and not the submodule. + /// + /// However, if we were to consider `from mypackage import myfunc` as introducing + /// the attribute `mypackage.myfunc` in `available_submodule_attributes`, we would + /// fail to ever resolve the function. This is because `available_submodule_attributes` + /// is *so early* and *so powerful* in our analysis that **this conclusion would be + /// used when actually resolving `from mypackage import myfunc`**! + /// + /// This currently cannot be fixed by considering the actual symbols defined in `mypackage`, + /// because `available_submodule_attributes` is an *input* to that analysis. + /// + /// We should therefore avoid marking something as an `available_submodule_attribute` + /// when the import could be importing a non-submodule (a function, class, or value). + /// + /// + /// # Rules + /// + /// We have two rules for whether a submodule attribute is defined: + /// + /// * 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 + /// *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?) 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)) }