mirror of
https://github.com/astral-sh/ruff.git
synced 2025-11-18 11:41:21 +00:00
[ty] support absolute from imports introducing local submodules in __init__.py files (#21372)
By resolving `.` and the LHS of the from import during semantic indexing, we can check if the LHS is a submodule of `.`, and handle `from whatever.thispackage.x.y import z` exactly like we do `from .x.y import z`. Fixes https://github.com/astral-sh/ty/issues/1484
This commit is contained in:
parent
44b0c9ebac
commit
bd8812127d
5 changed files with 79 additions and 68 deletions
|
|
@ -333,7 +333,7 @@ reveal_type(mypackage.nested.X) # revealed: Unknown
|
||||||
|
|
||||||
### In Non-Stub
|
### In Non-Stub
|
||||||
|
|
||||||
`from mypackage.submodule import nested` in an `__init__.py` only creates `nested`.
|
`from mypackage.submodule import nested` in an `__init__.py` creates both `submodule` and `nested`.
|
||||||
|
|
||||||
`mypackage/__init__.py`:
|
`mypackage/__init__.py`:
|
||||||
|
|
||||||
|
|
@ -357,12 +357,11 @@ X: int = 42
|
||||||
```py
|
```py
|
||||||
import mypackage
|
import mypackage
|
||||||
|
|
||||||
|
reveal_type(mypackage.submodule) # revealed: <module 'mypackage.submodule'>
|
||||||
# TODO: this would be nice to support
|
# TODO: this would be nice to support
|
||||||
# error: "has no member `submodule`"
|
# error: "has no member `nested`"
|
||||||
reveal_type(mypackage.submodule) # revealed: Unknown
|
|
||||||
# error: "has no member `submodule`"
|
|
||||||
reveal_type(mypackage.submodule.nested) # revealed: Unknown
|
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
|
reveal_type(mypackage.submodule.nested.X) # revealed: Unknown
|
||||||
reveal_type(mypackage.nested) # revealed: <module 'mypackage.submodule.nested'>
|
reveal_type(mypackage.nested) # revealed: <module 'mypackage.submodule.nested'>
|
||||||
reveal_type(mypackage.nested.X) # revealed: int
|
reveal_type(mypackage.nested.X) # revealed: int
|
||||||
|
|
|
||||||
|
|
@ -295,6 +295,7 @@ impl ModuleName {
|
||||||
Self::from_identifier_parts(db, importing_file, module.as_deref(), *level)
|
Self::from_identifier_parts(db, importing_file, module.as_deref(), *level)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Computes the absolute module name from the LHS components of `from LHS import RHS`
|
||||||
pub(crate) fn from_identifier_parts(
|
pub(crate) fn from_identifier_parts(
|
||||||
db: &dyn Db,
|
db: &dyn Db,
|
||||||
importing_file: File,
|
importing_file: File,
|
||||||
|
|
@ -309,6 +310,16 @@ impl ModuleName {
|
||||||
.ok_or(ModuleNameResolutionError::InvalidSyntax)
|
.ok_or(ModuleNameResolutionError::InvalidSyntax)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Computes the absolute module name for the package this file belongs to.
|
||||||
|
///
|
||||||
|
/// i.e. this resolves `.`
|
||||||
|
pub(crate) fn package_for_file(
|
||||||
|
db: &dyn Db,
|
||||||
|
importing_file: File,
|
||||||
|
) -> Result<Self, ModuleNameResolutionError> {
|
||||||
|
Self::from_identifier_parts(db, importing_file, None, 1)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Deref for ModuleName {
|
impl Deref for ModuleName {
|
||||||
|
|
|
||||||
|
|
@ -1451,7 +1451,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
|
||||||
|
|
||||||
// If we see:
|
// If we see:
|
||||||
//
|
//
|
||||||
// * `from .x.y import z` (must be relative!)
|
// * `from .x.y import z` (or `from whatever.thispackage.x.y`)
|
||||||
// * And we are in an `__init__.py(i)` (hereafter `thispackage`)
|
// * And we are in an `__init__.py(i)` (hereafter `thispackage`)
|
||||||
// * And this is the first time we've seen `from .x` in this module
|
// * And this is the first time we've seen `from .x` in this module
|
||||||
// * And we're in the global scope
|
// * And we're in the global scope
|
||||||
|
|
@ -1465,14 +1465,18 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
|
||||||
// reasons but it works well for most practical purposes. In particular it's nice
|
// 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
|
// that `x` can be freely overwritten, and that we don't assume that an import
|
||||||
// in one function is visible in another function.
|
// in one function is visible in another function.
|
||||||
//
|
if node.module.is_some()
|
||||||
// TODO: Also support `from thispackage.x.y import z`?
|
&& self.current_scope().is_global()
|
||||||
if self.current_scope() == FileScopeId::global()
|
|
||||||
&& node.level == 1
|
|
||||||
&& 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.file.is_package(self.db)
|
||||||
|
&& 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)
|
||||||
|
&& 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.seen_submodule_imports.contains(direct_submodule)
|
||||||
{
|
{
|
||||||
self.seen_submodule_imports
|
self.seen_submodule_imports
|
||||||
|
|
@ -1482,7 +1486,7 @@ impl<'ast> Visitor<'ast> for SemanticIndexBuilder<'_, 'ast> {
|
||||||
let symbol = self.add_symbol(direct_submodule_name);
|
let symbol = self.add_symbol(direct_submodule_name);
|
||||||
self.add_definition(
|
self.add_definition(
|
||||||
symbol.into(),
|
symbol.into(),
|
||||||
ImportFromSubmoduleDefinitionNodeRef { node, submodule },
|
ImportFromSubmoduleDefinitionNodeRef { node },
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -3,7 +3,6 @@ use std::ops::Deref;
|
||||||
use ruff_db::files::{File, FileRange};
|
use ruff_db::files::{File, FileRange};
|
||||||
use ruff_db::parsed::{ParsedModuleRef, parsed_module};
|
use ruff_db::parsed::{ParsedModuleRef, parsed_module};
|
||||||
use ruff_python_ast as ast;
|
use ruff_python_ast as ast;
|
||||||
use ruff_python_ast::name::Name;
|
|
||||||
use ruff_text_size::{Ranged, TextRange};
|
use ruff_text_size::{Ranged, TextRange};
|
||||||
|
|
||||||
use crate::Db;
|
use crate::Db;
|
||||||
|
|
@ -368,7 +367,6 @@ pub(crate) struct ImportFromDefinitionNodeRef<'ast> {
|
||||||
#[derive(Copy, Clone, Debug)]
|
#[derive(Copy, Clone, Debug)]
|
||||||
pub(crate) struct ImportFromSubmoduleDefinitionNodeRef<'ast> {
|
pub(crate) struct ImportFromSubmoduleDefinitionNodeRef<'ast> {
|
||||||
pub(crate) node: &'ast ast::StmtImportFrom,
|
pub(crate) node: &'ast ast::StmtImportFrom,
|
||||||
pub(crate) submodule: &'ast ast::Identifier,
|
|
||||||
}
|
}
|
||||||
#[derive(Copy, Clone, Debug)]
|
#[derive(Copy, Clone, Debug)]
|
||||||
pub(crate) struct AssignmentDefinitionNodeRef<'ast, 'db> {
|
pub(crate) struct AssignmentDefinitionNodeRef<'ast, 'db> {
|
||||||
|
|
@ -450,10 +448,8 @@ impl<'db> DefinitionNodeRef<'_, 'db> {
|
||||||
}),
|
}),
|
||||||
DefinitionNodeRef::ImportFromSubmodule(ImportFromSubmoduleDefinitionNodeRef {
|
DefinitionNodeRef::ImportFromSubmodule(ImportFromSubmoduleDefinitionNodeRef {
|
||||||
node,
|
node,
|
||||||
submodule,
|
|
||||||
}) => DefinitionKind::ImportFromSubmodule(ImportFromSubmoduleDefinitionKind {
|
}) => DefinitionKind::ImportFromSubmodule(ImportFromSubmoduleDefinitionKind {
|
||||||
node: AstNodeRef::new(parsed, node),
|
node: AstNodeRef::new(parsed, node),
|
||||||
submodule: submodule.as_str().into(),
|
|
||||||
}),
|
}),
|
||||||
DefinitionNodeRef::ImportStar(star_import) => {
|
DefinitionNodeRef::ImportStar(star_import) => {
|
||||||
let StarImportDefinitionNodeRef { node, symbol_id } = star_import;
|
let StarImportDefinitionNodeRef { node, symbol_id } = star_import;
|
||||||
|
|
@ -580,10 +576,7 @@ impl<'db> DefinitionNodeRef<'_, 'db> {
|
||||||
alias_index,
|
alias_index,
|
||||||
is_reexported: _,
|
is_reexported: _,
|
||||||
}) => (&node.names[alias_index]).into(),
|
}) => (&node.names[alias_index]).into(),
|
||||||
Self::ImportFromSubmodule(ImportFromSubmoduleDefinitionNodeRef {
|
Self::ImportFromSubmodule(ImportFromSubmoduleDefinitionNodeRef { node }) => node.into(),
|
||||||
node,
|
|
||||||
submodule: _,
|
|
||||||
}) => node.into(),
|
|
||||||
// INVARIANT: for an invalid-syntax statement such as `from foo import *, bar, *`,
|
// 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.
|
// we only create a `StarImportDefinitionKind` for the *first* `*` alias in the names list.
|
||||||
Self::ImportStar(StarImportDefinitionNodeRef { node, symbol_id: _ }) => node
|
Self::ImportStar(StarImportDefinitionNodeRef { node, symbol_id: _ }) => node
|
||||||
|
|
@ -1021,17 +1014,12 @@ impl ImportFromDefinitionKind {
|
||||||
#[derive(Clone, Debug, get_size2::GetSize)]
|
#[derive(Clone, Debug, get_size2::GetSize)]
|
||||||
pub struct ImportFromSubmoduleDefinitionKind {
|
pub struct ImportFromSubmoduleDefinitionKind {
|
||||||
node: AstNodeRef<ast::StmtImportFrom>,
|
node: AstNodeRef<ast::StmtImportFrom>,
|
||||||
submodule: Name,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ImportFromSubmoduleDefinitionKind {
|
impl ImportFromSubmoduleDefinitionKind {
|
||||||
pub fn import<'ast>(&self, module: &'ast ParsedModuleRef) -> &'ast ast::StmtImportFrom {
|
pub fn import<'ast>(&self, module: &'ast ParsedModuleRef) -> &'ast ast::StmtImportFrom {
|
||||||
self.node.node(module)
|
self.node.node(module)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn submodule(&self) -> &Name {
|
|
||||||
&self.submodule
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Clone, Debug, get_size2::GetSize)]
|
#[derive(Clone, Debug, get_size2::GetSize)]
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,6 @@ use itertools::{Either, Itertools};
|
||||||
use ruff_db::diagnostic::{Annotation, DiagnosticId, Severity};
|
use ruff_db::diagnostic::{Annotation, DiagnosticId, Severity};
|
||||||
use ruff_db::files::File;
|
use ruff_db::files::File;
|
||||||
use ruff_db::parsed::ParsedModuleRef;
|
use ruff_db::parsed::ParsedModuleRef;
|
||||||
use ruff_python_ast::name::Name;
|
|
||||||
use ruff_python_ast::visitor::{Visitor, walk_expr};
|
use ruff_python_ast::visitor::{Visitor, walk_expr};
|
||||||
use ruff_python_ast::{
|
use ruff_python_ast::{
|
||||||
self as ast, AnyNodeRef, ExprContext, HasNodeIndex, NodeIndex, PythonVersion,
|
self as ast, AnyNodeRef, ExprContext, HasNodeIndex, NodeIndex, PythonVersion,
|
||||||
|
|
@ -1218,7 +1217,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
DefinitionKind::ImportFromSubmodule(import_from) => {
|
DefinitionKind::ImportFromSubmodule(import_from) => {
|
||||||
self.infer_import_from_submodule_definition(
|
self.infer_import_from_submodule_definition(
|
||||||
import_from.import(self.module()),
|
import_from.import(self.module()),
|
||||||
import_from.submodule(),
|
|
||||||
definition,
|
definition,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -5901,51 +5899,64 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Infer the implicit local definition `x = <module 'thispackage.x'>` that
|
/// Infer the implicit local definition `x = <module 'whatever.thispackage.x'>` that
|
||||||
/// `from .x.y import z` can introduce in an `__init__.py(i)`.
|
/// `from .x.y import z` or `from whatever.thispackage.x.y` can introduce in `__init__.py(i)`.
|
||||||
///
|
///
|
||||||
/// For the definition `z`, see [`TypeInferenceBuilder::infer_import_from_definition`].
|
/// For the definition `z`, see [`TypeInferenceBuilder::infer_import_from_definition`].
|
||||||
|
///
|
||||||
|
/// The runtime semantic of this kind of statement is to introduce a variable in the global
|
||||||
|
/// scope of this module *the first time it's imported in the entire program*. This
|
||||||
|
/// implementation just blindly introduces a local variable wherever the `from..import` is
|
||||||
|
/// (if the imports actually resolve).
|
||||||
|
///
|
||||||
|
/// That gap between the semantics and implementation are currently the responsibility of the
|
||||||
|
/// code that actually creates these kinds of Definitions (so blindly introducing a local
|
||||||
|
/// is all we need to be doing here).
|
||||||
fn infer_import_from_submodule_definition(
|
fn infer_import_from_submodule_definition(
|
||||||
&mut self,
|
&mut self,
|
||||||
import_from: &ast::StmtImportFrom,
|
import_from: &ast::StmtImportFrom,
|
||||||
submodule: &Name,
|
|
||||||
definition: Definition<'db>,
|
definition: Definition<'db>,
|
||||||
) {
|
) {
|
||||||
// The runtime semantic of this kind of statement is to introduce a variable in the global
|
// Get this package's absolute module name by resolving `.`, and make sure it exists
|
||||||
// scope of this module, so we do just that. (Actually we introduce a local variable, but
|
let Ok(thispackage_name) = ModuleName::package_for_file(self.db(), self.file()) else {
|
||||||
// this type of Definition is only created when a `from..import` is in global scope.)
|
self.add_binding(import_from.into(), definition, |_, _| Type::unknown());
|
||||||
|
return;
|
||||||
// Get this package's module by resolving `.`
|
};
|
||||||
let Ok(module_name) = ModuleName::from_identifier_parts(self.db(), self.file(), None, 1)
|
let Some(module) = resolve_module(self.db(), &thispackage_name) else {
|
||||||
else {
|
|
||||||
self.add_binding(import_from.into(), definition, |_, _| Type::unknown());
|
self.add_binding(import_from.into(), definition, |_, _| Type::unknown());
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
|
||||||
let Some(module) = resolve_module(self.db(), &module_name) else {
|
// We have `from whatever.thispackage.x.y ...` or `from .x.y ...`
|
||||||
|
// and we want to extract `x` (to ultimately construct `whatever.thispackage.x`):
|
||||||
|
|
||||||
|
// First we normalize to `whatever.thispackage.x.y`
|
||||||
|
let Some(final_part) = ModuleName::from_identifier_parts(
|
||||||
|
self.db(),
|
||||||
|
self.file(),
|
||||||
|
import_from.module.as_deref(),
|
||||||
|
import_from.level,
|
||||||
|
)
|
||||||
|
.ok()
|
||||||
|
// `whatever.thispackage.x.y` => `x.y`
|
||||||
|
.and_then(|submodule_name| submodule_name.relative_to(&thispackage_name))
|
||||||
|
// `x.y` => `x`
|
||||||
|
.and_then(|relative_submodule_name| {
|
||||||
|
relative_submodule_name
|
||||||
|
.components()
|
||||||
|
.next()
|
||||||
|
.and_then(ModuleName::new)
|
||||||
|
}) else {
|
||||||
self.add_binding(import_from.into(), definition, |_, _| Type::unknown());
|
self.add_binding(import_from.into(), definition, |_, _| Type::unknown());
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
|
||||||
// Now construct the submodule `.x`
|
// `x` => `whatever.thispackage.x`
|
||||||
assert!(
|
let mut full_submodule_name = thispackage_name.clone();
|
||||||
!submodule.is_empty(),
|
full_submodule_name.extend(&final_part);
|
||||||
"ImportFromSubmoduleDefinitionKind constructed with empty module"
|
|
||||||
);
|
// Try to actually resolve the import `whatever.thispackage.x`
|
||||||
let name = submodule
|
if let Some(submodule_type) = self.module_type_from_name(&full_submodule_name) {
|
||||||
.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
|
|
||||||
});
|
|
||||||
// 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!
|
// Success, introduce a binding!
|
||||||
//
|
//
|
||||||
// We explicitly don't introduce a *declaration* because it's actual ok
|
// We explicitly don't introduce a *declaration* because it's actual ok
|
||||||
|
|
@ -5970,17 +5981,15 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
};
|
};
|
||||||
|
|
||||||
let diagnostic = builder.into_diagnostic(format_args!(
|
let diagnostic = builder.into_diagnostic(format_args!(
|
||||||
"Module `{module_name}` has no submodule `{name}`"
|
"Module `{thispackage_name}` has no submodule `{final_part}`"
|
||||||
));
|
));
|
||||||
|
|
||||||
if let Some(full_submodule_name) = full_submodule_name {
|
hint_if_stdlib_submodule_exists_on_other_versions(
|
||||||
hint_if_stdlib_submodule_exists_on_other_versions(
|
self.db(),
|
||||||
self.db(),
|
diagnostic,
|
||||||
diagnostic,
|
&full_submodule_name,
|
||||||
&full_submodule_name,
|
module,
|
||||||
module,
|
);
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn infer_return_statement(&mut self, ret: &ast::StmtReturn) {
|
fn infer_return_statement(&mut self, ret: &ast::StmtReturn) {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue