Merge 961453f75e into b0bc990cbf

crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md

@@ -647,8 +647,8 @@ reveal_type(mypackage.imported.X)  # revealed: int
 
 ## `from` Import of Other Package's Submodule
 
-`from mypackage import submodule` from outside the package is not modeled as a side-effect on
+`from mypackage import submodule` and `from mypackage.submodule import not_a_submodule` from outside
-`mypackage`, even in the importing file (this could be changed!).
+the package are both modeled as a side-effects on `mypackage`.
 
 ### In Stub
 
@@ -663,18 +663,28 @@ reveal_type(mypackage.imported.X)  # revealed: int
 X: int = 42
 ```
 
+`package2/__init__.pyi`:
+
+```pyi
+```
+
+`package2/submodule.pyi`:
+
+```pyi
+not_a_submodule: int
+```
+
 `main.py`:
 
 ```py
 import mypackage
+import package2
 from mypackage import imported
+from package2.submodule import not_a_submodule
 
 reveal_type(imported.X)  # revealed: int
-
+reveal_type(mypackage.imported.X)  # revealed: int
-# TODO: this would be nice to support, but it's dangerous with available_submodule_attributes
+reveal_type(package2.submodule.not_a_submodule)  # revealed: int
-# for details, see: https://github.com/astral-sh/ty/issues/1488
-# error: [possibly-missing-attribute] "Submodule `imported` may not be available"
-reveal_type(mypackage.imported.X)  # revealed: Unknown
 ```
 
 ### In Non-Stub
@@ -690,17 +700,28 @@ reveal_type(mypackage.imported.X)  # revealed: Unknown
 X: int = 42
 ```
 
+`package2/__init__.py`:
+
+```py
+```
+
+`package2/submodule.py`:
+
+```py
+not_a_submodule: int
+```
+
 `main.py`:
 
 ```py
 import mypackage
+import package2
 from mypackage import imported
+from package2.submodule import not_a_submodule
 
 reveal_type(imported.X)  # revealed: int
-
+reveal_type(mypackage.imported.X)  # revealed: int
-# TODO: this would be nice to support, as it works at runtime
+reveal_type(package2.submodule.not_a_submodule)  # revealed: int
-# error: [possibly-missing-attribute] "Submodule `imported` may not be available"
-reveal_type(mypackage.imported.X)  # revealed: Unknown
 ```
 
 ## `from` Import of Sibling Module
@@ -859,6 +880,51 @@ from mypackage import funcmod
 x = funcmod(1)
 ```
 
+## A Tale of Two Modules
+
+This is a nonsensical regression test for some incredibly cursed interaction in `ty` where we get
+confused about `mypackage.conflicted`. The worst part is that resolving an import
+`from typing import <anything that resolves>` is load-bearing, and `typing` seems to be special
+here. There is no known reason why `typing` should be special here.
+
+### In Stub
+
+`mypackage/__init__.py`:
+
+```py
+from .conflicted.b import x
+```
+
+`mypackage/conflicted/__init__.py`:
+
+`mypackage/conflicted/other1/__init__.py`:
+
+```py
+x: int = 1
+```
+
+`mypackage/conflicted/b/__init__.py`:
+
+```py
+x: int = 1
+```
+
+`mypackage/conflicted/b/c/__init__.py`:
+
+```py
+y: int = 2
+```
+
+`main.py`:
+
+```py
+from typing import TYPE_CHECKING
+from mypackage.conflicted.other1 import x as x1
+import mypackage.conflicted.b.c
+
+reveal_type(mypackage.conflicted.b.c.y)  # revealed: int
+```
+
 ## Re-export Nameclash Problems In Functions
 
 `from` imports in an `__init__.py` at file scope should be visible to functions defined in the file:

crates/ty_python_semantic/resources/mdtest/import/tracking.md

@@ -116,3 +116,43 @@ b = 1
 
 ```py
 ```
+
+## Submodule is loaded in a non-global scope
+
+We recognise submodules as being available as attributes even if they are loaded in a function
+scope. The function might never be executed, which means that the submodule might never be loaded;
+however, we prefer to prioritise avoiding false positives over catching all possible errors here.
+
+`a/b.py`:
+
+```py
+```
+
+`a/c.py`:
+
+```py
+d = 42
+```
+
+`a/e/f.py`:
+
+```py
+```
+
+`main.py`:
+
+```py
+import a
+
+def f():
+    import a.b
+    from a.c import d
+    from a.e import f
+
+f()
+
+reveal_type(a.b)  # revealed: <module 'a.b'>
+reveal_type(a.c)  # revealed: <module 'a.c'>
+reveal_type(a.e)  # revealed: <module 'a.e'>
+reveal_type(a.e.f)  # revealed: <module 'a.e.f'>
+```

crates/ty_python_semantic/src/semantic_index.rs

@@ -79,7 +79,10 @@ pub(crate) fn place_table<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> Arc<Plac
 /// See [`ModuleLiteralType::available_submodule_attributes`] for discussion
 /// of why this analysis is intentionally limited.
 #[salsa::tracked(returns(deref), heap_size=ruff_memory_usage::heap_size)]
-pub(crate) fn imported_modules<'db>(db: &'db dyn Db, file: File) -> Arc<FxHashSet<ModuleName>> {
+pub(crate) fn imported_modules<'db>(
+    db: &'db dyn Db,
+    file: File,
+) -> Arc<FxHashMap<ModuleName, ImportKind>> {
     semantic_index(db, file).imported_modules.clone()
 }
 
@@ -246,7 +249,7 @@ pub(crate) struct SemanticIndex<'db> {
     ast_ids: IndexVec<FileScopeId, AstIds>,
 
     /// The set of modules that are imported anywhere within this file.
-    imported_modules: Arc<FxHashSet<ModuleName>>,
+    imported_modules: Arc<FxHashMap<ModuleName, ImportKind>>,
 
     /// Flags about the global scope (code usage impacting inference)
     has_future_annotations: bool,
@@ -588,6 +591,12 @@ impl<'db> SemanticIndex<'db> {
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq, get_size2::GetSize)]
+pub(crate) enum ImportKind {
+    Import,
+    ImportFrom,
+}
+
 pub(crate) struct AncestorsIter<'a> {
     scopes: &'a IndexSlice<FileScopeId, Scope>,
     next_id: Option<FileScopeId>,

crates/ty_python_semantic/src/semantic_index/builder.rs

@@ -47,7 +47,7 @@ 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, ImportKind, SemanticIndex, VisibleAncestorsIter};
 use crate::semantic_model::HasTrackedScope;
 use crate::unpack::{EvaluationMode, Unpack, UnpackKind, UnpackPosition, UnpackValue};
 use crate::{Db, Program};
@@ -110,7 +110,7 @@ pub(super) struct SemanticIndexBuilder<'db, 'ast> {
     scopes_by_expression: ExpressionsScopeMapBuilder,
     definitions_by_node: FxHashMap<DefinitionNodeKey, Definitions<'db>>,
     expressions_by_node: FxHashMap<ExpressionNodeKey, Expression<'db>>,
-    imported_modules: FxHashSet<ModuleName>,
+    imported_modules: FxHashMap<ModuleName, ImportKind>,
     seen_submodule_imports: FxHashSet<String>,
     /// Hashset of all [`FileScopeId`]s that correspond to [generator functions].
     ///
@@ -150,7 +150,7 @@ impl<'db, 'ast> SemanticIndexBuilder<'db, 'ast> {
             expressions_by_node: FxHashMap::default(),
 
             seen_submodule_imports: FxHashSet::default(),
-            imported_modules: FxHashSet::default(),
+            imported_modules: FxHashMap::default(),
             generator_functions: FxHashSet::default(),
 
             enclosing_snapshots: FxHashMap::default(),
@@ -1474,7 +1474,11 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
                     // Mark the imported module, and all of its parents, as being imported in this
                     // file.
                     if let Some(module_name) = ModuleName::new(&alias.name) {
-                        self.imported_modules.extend(module_name.ancestors());
+                        self.imported_modules.extend(
+                            module_name
+                                .ancestors()
+                                .zip(std::iter::repeat(ImportKind::Import)),
+                        );
                     }
 
                     let (symbol_name, is_reexported) = if let Some(asname) = &alias.asname {
@@ -1517,23 +1521,27 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
                 // that `x` can be freely overwritten, and that we don't assume that an import
                 // in one function is visible in another function.
                 let mut is_self_import = false;
-                if self.file.is_package(self.db)
+                let is_package = self.file.is_package(self.db);
-                    && let Ok(module_name) = ModuleName::from_identifier_parts(
+                let this_package = ModuleName::package_for_file(self.db, self.file);
+
+                if let Ok(module_name) = ModuleName::from_identifier_parts(
                     self.db,
                     self.file,
                     node.module.as_deref(),
                     node.level,
-                    )
+                ) {
-                    && let Ok(thispackage) = ModuleName::package_for_file(self.db, self.file)
-                {
                     // Record whether this is equivalent to `from . import ...`
-                    is_self_import = module_name == thispackage;
+                    if is_package && let Ok(thispackage) = this_package.as_ref() {
+                        is_self_import = &module_name == thispackage;
+                    }
 
-                    if node.module.is_some()
+                    if node.module.is_some() {
+                        if is_package
+                            && let Ok(thispackage) = this_package
+                            && self.current_scope().is_global()
                             && let Some(relative_submodule) = module_name.relative_to(&thispackage)
                             && let Some(direct_submodule) = relative_submodule.components().next()
                             && !self.seen_submodule_imports.contains(direct_submodule)
-                        && self.current_scope().is_global()
                         {
                             self.seen_submodule_imports
                                 .insert(direct_submodule.to_owned());
@@ -1544,6 +1552,23 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
                                 symbol.into(),
                                 ImportFromSubmoduleDefinitionNodeRef { node },
                             );
+                        } else {
+                            for name in module_name.ancestors() {
+                                self.imported_modules
+                                    .entry(name)
+                                    .or_insert(ImportKind::ImportFrom);
+                            }
+                            for name in &node.names {
+                                let Some(relative_name) = ModuleName::new(&name.name) else {
+                                    continue;
+                                };
+                                let mut full_name = module_name.clone();
+                                full_name.extend(&relative_name);
+                                self.imported_modules
+                                    .entry(full_name)
+                                    .or_insert(ImportKind::ImportFrom);
+                            }
+                        }
                     }
                 }
 

crates/ty_python_semantic/src/types.rs

@@ -42,7 +42,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, place_table, semantic_index};
+use crate::semantic_index::{ImportKind, imported_modules, place_table, semantic_index};
 use crate::suppression::check_suppressions;
 use crate::types::bound_super::BoundSuperType;
 use crate::types::builder::RecursivelyDefined;
@@ -13371,12 +13371,18 @@ impl<'db> ModuleLiteralType<'db> {
     ///
     /// 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<Item = Name> {
+    fn available_submodule_attributes(
+        &self,
+        db: &'db dyn Db,
+    ) -> impl Iterator<Item = (Name, ImportKind)> {
         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)))
+            .filter_map(|(submodule_name, kind)| {
-            .filter_map(|relative_submodule| relative_submodule.components().next().map(Name::from))
+                let relative_name = submodule_name.relative_to(self.module(db).name(db))?;
+                let available_attribute = relative_name.components().next()?;
+                Some((Name::from(available_attribute), *kind))
+            })
     }
 
     fn resolve_submodule(self, db: &'db dyn Db, name: &str) -> Option<Type<'db>> {
@@ -13420,33 +13426,55 @@ impl<'db> ModuleLiteralType<'db> {
                 .member(db, "__dict__");
         }
 
-        // If the file that originally imported the module has also imported a submodule
+        let mut submodule_type = None;
-        // named `name`, then the result is (usually) that submodule, even if the module
+
-        // also defines a (non-module) symbol with that name.
+        let available_submodule_kind = self
-        //
+            .available_submodule_attributes(db)
-        // Note that technically, either the submodule or the non-module symbol could take
+            .find_map(|(attr, kind)| (attr == name).then_some(kind));
-        // priority, depending on the ordering of when the submodule is loaded relative to
+
-        // the parent module's `__init__.py` file being evaluated. That said, we have
+        if available_submodule_kind.is_some() {
-        // chosen to always have the submodule take priority. (This matches pyright's
+            submodule_type = self.resolve_submodule(db, name);
-        // current behavior, but is the opposite of mypy's current behavior.)
-        if self.available_submodule_attributes(db).contains(name) {
-            if let Some(submodule) = self.resolve_submodule(db, name) {
-                return Place::bound(submodule).into();
         }
+
+        // if we're in a module `foo` and `foo` contains `import a.b`,
+        // and the package `a` has a submodule `b`, we assume that the
+        // attribute access `a.b` inside `foo` will resolve to the submodule
+        // `a.b` *even if* `a/__init__.py` also defines a symbol `b` (e.g. `b = 42`).
+        // This is a heuristic, but it's almost certainly what will actually happen
+        // at runtime. However, if `foo` only contains `from a.b import <something>,
+        // we prioritise the `b` attribute in `a/__init__.py` over the submodule `a.b`.
+        if available_submodule_kind == Some(ImportKind::Import)
+            && let Some(submodule) = submodule_type
+        {
+            return Place::bound(submodule).into();
         }
 
         let place_and_qualifiers = self
             .module(db)
             .file(db)
-            .map(|file| imported_symbol(db, file, name, None))
+            .map(|file| {
+                imported_symbol(db, file, name, None).map_type(|ty| {
+                    if let Some(importing) = self.importing_file(db)
+                        && let Type::ModuleLiteral(module) = ty
+                    {
+                        Type::module_literal(db, importing, module.module(db))
+                    } else {
+                        ty
+                    }
+                })
+            })
             .unwrap_or_default();
 
-        // If the normal lookup failed, try to call the module's `__getattr__` function
+        if !place_and_qualifiers.is_undefined() {
-        if place_and_qualifiers.place.is_undefined() {
+            return place_and_qualifiers;
-            return self.try_module_getattr(db, name);
         }
 
-        place_and_qualifiers
+        if let Some(submodule) = submodule_type {
+            return Place::bound(submodule).into();
+        }
+
+        // If the normal lookup failed, try to call the module's `__getattr__` function
+        self.try_module_getattr(db, name)
     }
 }
 

crates/ty_python_semantic/src/types/list_members.rs

@@ -371,7 +371,7 @@ impl<'db> AllMembers<'db> {
 
                 self.members
                     .extend(literal.available_submodule_attributes(db).filter_map(
-                        |submodule_name| {
+                        |(submodule_name, _)| {
                             let ty = literal.resolve_submodule(db, &submodule_name)?;
                             let name = submodule_name.clone();
                             Some(Member { name, ty })

playground/package-lock.json

@@ -1780,7 +1780,6 @@
       "integrity": "sha512-AwAfQ2Wa5bCx9WP8nZL2uMZWod7J7/JSplxbTmBQ5ms6QpqNYm672H0Vu9ZVKVngQ+ii4R/byguVEUZQyeg44g==",
       "dev": true,
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "csstype": "^3.0.2"
       }
@@ -1841,7 +1840,6 @@
       "integrity": "sha512-Zhy8HCvBUEfBECzIl1PKqF4p11+d0aUJS1GeUiuqK9WmOug8YCmC4h4bjyBvMyAMI9sbRczmrYL5lKg/YMbrcQ==",
       "dev": true,
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "@typescript-eslint/scope-manager": "8.38.0",
         "@typescript-eslint/types": "8.38.0",
@@ -2087,7 +2085,6 @@
       "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==",
       "dev": true,
       "license": "MIT",
-      "peer": true,
       "bin": {
         "acorn": "bin/acorn"
       },
@@ -3004,7 +3001,6 @@
       "integrity": "sha512-LSehfdpgMeWcTZkWZVIJl+tkZ2nuSkyyB9C27MZqFWXuph7DvaowgcTvKqxvpLW1JZIk8PN7hFY3Rj9LQ7m7lg==",
       "dev": true,
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "@eslint-community/eslint-utils": "^4.2.0",
         "@eslint-community/regexpp": "^4.12.1",
@@ -4863,7 +4859,6 @@
       "resolved": "https://registry.npmjs.org/monaco-editor/-/monaco-editor-0.54.0.tgz",
       "integrity": "sha512-hx45SEUoLatgWxHKCmlLJH81xBo0uXP4sRkESUpmDQevfi+e7K1VuiSprK6UpQ8u4zOcKNiH0pMvHvlMWA/4cw==",
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "dompurify": "3.1.7",
         "marked": "14.0.0"
@@ -5322,7 +5317,6 @@
       "resolved": "https://registry.npmjs.org/react/-/react-19.1.1.tgz",
       "integrity": "sha512-w8nqGImo45dmMIfljjMwOGtbmC/mk4CMYhWIicdSflH91J9TyCyczcPFXJzrZ/ZXcgGRFeP6BU0BEJTw6tZdfQ==",
       "license": "MIT",
-      "peer": true,
       "engines": {
         "node": ">=0.10.0"
       }
@@ -5332,7 +5326,6 @@
       "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.1.1.tgz",
       "integrity": "sha512-Dlq/5LAZgF0Gaz6yiqZCf6VCcZs1ghAJyrsu84Q/GT0gV+mCxbfmKNoGRKBYMJ8IEdGPqu49YWXD02GCknEDkw==",
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "scheduler": "^0.26.0"
       },
@@ -6030,7 +6023,6 @@
       "integrity": "sha512-M7BAV6Rlcy5u+m6oPhAPFgJTzAioX/6B0DxyvDlo9l8+T3nLKbrczg2WLUyzd45L8RqfUMyGPzekbMvX2Ldkwg==",
       "dev": true,
       "license": "MIT",
-      "peer": true,
       "engines": {
         "node": ">=12"
       },
@@ -6182,7 +6174,6 @@
       "integrity": "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ==",
       "dev": true,
       "license": "Apache-2.0",
-      "peer": true,
       "bin": {
         "tsc": "bin/tsc",
         "tsserver": "bin/tsserver"
@@ -6260,7 +6251,6 @@
       "integrity": "sha512-cJBdq0/u+8rgstg9t7UkBilf8ipLmeXJO30NxD5HAHOivnj10ocV8YtR/XBvd2wQpN3TmcaxNKaHX3tN7o5F5A==",
       "dev": true,
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "esbuild": "^0.25.0",
         "fdir": "^6.4.6",
@@ -6374,7 +6364,6 @@
       "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==",
       "dev": true,
       "license": "MIT",
-      "peer": true,
       "engines": {
         "node": ">=12"
       },