[ty] Retain package listings between resolution steps (#28278)
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
requests.)
- Does this pull request include references to any relevant issues?
- Does this PR follow our AI policy
(https://github.com/astral-sh/.github/blob/main/AI_POLICY.md)?
-->
## Summary
This retains directory listings between successive steps of module
resolution, using the `ModuleDirectory` abstraction introduced in
[#28418](https://github.com/astral-sh/ruff/pull/28418).
For example, when resolving `acme.tools`:
```text
acme/
├── __init__.py
└── tools.py
```
Resolving `acme` retrieves its directory listing to find `__init__.py`.
Resolving the next component, `tools`, needs that same listing.
Previously, each resolution candidate retained only the directory’s
path, so the next step reconstructed a `ModuleDirectory` and retrieved
the listing again.
Candidates now retain the `ModuleDirectory` itself, preserving both its
path and its listing, so the resolution for `acme` and `acme.tools` can
both reuse the directory listing for `acme/`. Note that directory
listings are already cached with Salsa, but this avoids repeated cache
retrieval and the path construction needed to perform that retrieval.
Resolution rules remain unchanged.
Downstream, retaining these listings will also allow namespace-package
enumeration to discover child names and resolve them using the same
directory information, as part of [namespace-package auto-import
support](https://github.com/astral-sh/ty/issues/2273).
## Test Plan
This is a behaviour-preserving refactor that relies on existing tests.
<!-- How was it tested? -->
diff --git a/crates/ty_module_resolver/src/path.rs b/crates/ty_module_resolver/src/path.rs
index f1b3208..2494cbd 100644
--- a/crates/ty_module_resolver/src/path.rs
+++ b/crates/ty_module_resolver/src/path.rs
@@ -284,10 +284,6 @@
}
}
}
-
- pub(crate) fn into_search_path(self) -> SearchPath {
- self.search_path
- }
}
impl PartialEq<SystemPathBuf> for ModulePath {
@@ -364,6 +360,11 @@
&self.path
}
+ /// Consumes the directory and returns its search root.
+ pub(crate) fn into_search_path(self) -> SearchPath {
+ self.path.search_path
+ }
+
/// Returns the cached listing from [`System`], or `None` for an inaccessible directory
/// or a path in the vendored typeshed archive.
pub(crate) fn system_listing(&self) -> Option<&'db DirectoryListing> {
diff --git a/crates/ty_module_resolver/src/resolve.rs b/crates/ty_module_resolver/src/resolve.rs
index 7235e05..75da76c 100644
--- a/crates/ty_module_resolver/src/resolve.rs
+++ b/crates/ty_module_resolver/src/resolve.rs
@@ -1203,7 +1203,7 @@
resolver_environment: ResolverEnvironment<'db>,
name: &ModuleName,
mode: ModuleResolveMode,
-) -> Option<ResolvedNames> {
+) -> Option<ResolvedNames<'db>> {
let resolver = NameResolver::new(db, resolver_environment, name, mode);
match mode {
@@ -1226,7 +1226,7 @@
resolver_environment: ResolverEnvironment<'db>,
name: &ModuleName,
mode: ModuleResolveMode,
-) -> Option<ResolvedNames> {
+) -> Option<ResolvedNames<'db>> {
let importing_file = ResolverFile::new(db, importing_file, resolver_environment);
let search_paths = absolute_desperate_search_paths(db, importing_file).unwrap_or_default();
let resolver = NameResolver::new(db, resolver_environment, name, mode);
@@ -1277,25 +1277,33 @@
}
#[derive(Debug, Clone)]
-struct ModuleResolutionCandidate {
- path: ModulePath,
+struct ModuleResolutionCandidate<'db> {
+ // This represents the directory containing a file-module candidate, or the
+ // root directory of a package candidate. It is initialized to the search-path
+ // root, and is subsequently updated as each part of the module name is resolved.
+ // The resolved file, if any, is stored in `module`.
+ directory: ModuleDirectory<'db>,
module: ResolvedModule,
py_typed: PyTyped,
precedence: CandidatePrecedence,
}
-impl ModuleResolutionCandidate {
- fn root(search_path: &SearchPath) -> Self {
- Self::with_precedence(search_path, CandidatePrecedence::SearchPathOrder)
+impl<'db> ModuleResolutionCandidate<'db> {
+ fn root(context: &ResolverContext<'db>, search_path: &SearchPath) -> Self {
+ Self::with_precedence(context, search_path, CandidatePrecedence::SearchPathOrder)
}
- fn stub(search_path: &SearchPath) -> Self {
- Self::with_precedence(search_path, CandidatePrecedence::StubPackage)
+ fn stub(context: &ResolverContext<'db>, search_path: &SearchPath) -> Self {
+ Self::with_precedence(context, search_path, CandidatePrecedence::StubPackage)
}
- fn with_precedence(search_path: &SearchPath, precedence: CandidatePrecedence) -> Self {
+ fn with_precedence(
+ context: &ResolverContext<'db>,
+ search_path: &SearchPath,
+ precedence: CandidatePrecedence,
+ ) -> Self {
Self {
- path: search_path.to_module_path(),
+ directory: ModuleDirectory::new(context, search_path.to_module_path()),
module: ResolvedModule::NamespacePackage,
py_typed: PyTyped::Untyped,
precedence,
@@ -1313,7 +1321,7 @@
}
// This is the module we were actually interested in resolving, complete the resolution
- fn into_module<'db>(
+ fn into_module(
self,
db: &'db dyn Db,
resolver_environment: ResolverEnvironment<'db>,
@@ -1337,7 +1345,7 @@
resolver_environment,
Cow::Borrowed(name),
ModuleKind::Package,
- self.path.into_search_path(),
+ self.directory.into_search_path(),
)
}
ResolvedModule::RegularPackage(file) => {
@@ -1351,7 +1359,7 @@
resolver_environment,
Cow::Borrowed(name),
ModuleKind::Package,
- self.path.into_search_path(),
+ self.directory.into_search_path(),
)
}
ResolvedModule::Module(file) => {
@@ -1362,7 +1370,7 @@
resolver_environment,
Cow::Borrowed(name),
ModuleKind::Module,
- self.path.into_search_path(),
+ self.directory.into_search_path(),
)
}
}
@@ -1385,9 +1393,13 @@
fn to_str<'a>(&self, db: &'a dyn Db) -> Cow<'a, str> {
match self.module {
- ResolvedModule::NamespacePackage => {
- Cow::Owned(self.path.to_system_path().unwrap_or_default().to_string())
- }
+ ResolvedModule::NamespacePackage => Cow::Owned(
+ self.directory
+ .path()
+ .to_system_path()
+ .unwrap_or_default()
+ .to_string(),
+ ),
ResolvedModule::LegacyNamespacePackage(file) => Cow::Borrowed(file.path(db).as_str()),
ResolvedModule::RegularPackage(file) => Cow::Borrowed(file.path(db).as_str()),
ResolvedModule::Module(file) => Cow::Borrowed(file.path(db).as_str()),
@@ -1421,7 +1433,7 @@
/// This includes PEP 561 stub packages and user-provided stub overlays, with runtime source as
/// a fallback when no stub provides the requested module. A stub overlay may use runtime
/// packages as parents, but its final module must come from a stub file.
- fn resolve_typing(&self, stub_packages: &StubPackageIndex) -> Option<ResolvedNames> {
+ fn resolve_typing(&self, stub_packages: &StubPackageIndex) -> Option<ResolvedNames<'db>> {
if self.name.components().nth(1).is_none() {
let candidates = self.discover_roots(
search_paths(
@@ -1472,7 +1484,7 @@
/// These paths can contain PEP 561 stub packages, but never user-provided extra paths, so this
/// indexes them for stub packages without performing a separate stub-overlay pass. Runtime
/// resolution instead ignores stub packages and `.pyi` files entirely.
- fn resolve_desperate_typing(&self, search_paths: &[SearchPath]) -> Option<ResolvedNames> {
+ fn resolve_desperate_typing(&self, search_paths: &[SearchPath]) -> Option<ResolvedNames<'db>> {
let stub_packages =
StubPackageIndex::from_search_paths(self.context.db, search_paths.iter());
let candidates = self.discover_roots(search_paths.iter(), stub_packages.all());
@@ -1486,7 +1498,7 @@
fn resolve_runtime<'a>(
&self,
search_paths: impl Iterator<Item = &'a SearchPath>,
- ) -> Option<ResolvedNames> {
+ ) -> Option<ResolvedNames<'db>> {
let candidates = self.discover_roots(search_paths, StubPackagePaths::default());
self.resolve_remaining(candidates, ComponentFileFilter::ByMode)
}
@@ -1495,7 +1507,7 @@
&self,
search_paths: impl Iterator<Item = &'a SearchPath>,
stub_paths: StubPackagePaths<'_>,
- ) -> ResolvedNames {
+ ) -> ResolvedNames<'db> {
let root_component = self.name.first_component();
let mut cur_candidates = Vec::new();
let stub_name = (!stub_paths.is_empty() && !self.is_non_shadowable)
@@ -1527,7 +1539,7 @@
// A terminal candidate can stop the search unless a matching post-stdlib stub package
// could still override it. A terminal stdlib candidate always stops the search.
let can_stop = is_stdlib || pending_stub_paths.is_empty();
- let mut candidate = ModuleResolutionCandidate::root(search_path);
+ let mut candidate = ModuleResolutionCandidate::root(&self.context, search_path);
let resolved = resolve_component(
&self.context,
&mut candidate,
@@ -1559,9 +1571,9 @@
fn resolve_remaining(
&self,
- mut cur_candidates: ResolvedNames,
+ mut cur_candidates: ResolvedNames<'db>,
final_filter: ComponentFileFilter,
- ) -> Option<ResolvedNames> {
+ ) -> Option<ResolvedNames<'db>> {
if cur_candidates.is_empty() {
return None;
}
@@ -1608,12 +1620,12 @@
}
}
-fn resolve_stub_package_in_search_path(
- context: &ResolverContext,
+fn resolve_stub_package_in_search_path<'db>(
+ context: &ResolverContext<'db>,
search_path: &SearchPath,
stub_name: &str,
-) -> Option<ModuleResolutionCandidate> {
- let mut candidate = ModuleResolutionCandidate::stub(search_path);
+) -> Option<ModuleResolutionCandidate<'db>> {
+ let mut candidate = ModuleResolutionCandidate::stub(context, search_path);
resolve_component(
context,
&mut candidate,
@@ -1634,11 +1646,11 @@
}
}
-fn normalize_candidates(
+fn normalize_candidates<'db>(
db: &dyn Db,
- mut candidates: ResolvedNames,
+ mut candidates: ResolvedNames<'db>,
has_remaining_components: bool,
-) -> ResolvedNames {
+) -> ResolvedNames<'db> {
let best_concrete_precedence = candidates
.iter()
.filter(|candidate| !candidate.is_any_namespace_package())
@@ -1685,9 +1697,9 @@
}
/// Resolves one component relative to the candidate's current package.
-fn resolve_component(
- context: &ResolverContext,
- candidate: &mut ModuleResolutionCandidate,
+fn resolve_component<'db>(
+ context: &ResolverContext<'db>,
+ candidate: &mut ModuleResolutionCandidate<'db>,
module_name: &str,
file_filter: ComponentFileFilter,
) -> Result<(), ()> {
@@ -1699,35 +1711,34 @@
return Err(());
}
- let module_directory = ModuleDirectory::new(context, candidate.path.clone());
+ let module_directory = &candidate.directory;
if !module_directory.may_contain_name(module_name) {
return Err(());
}
let subdirectory = module_directory.child_directory(context, module_name);
- let init = subdirectory
- .as_ref()
- .and_then(|dir| resolve_file_module_with_filter(dir, context, "__init__", file_filter));
- candidate.path.push(module_name);
- if let Some(init) = init {
+ if let Some(subdirectory) = &subdirectory
+ && let Some(init) =
+ resolve_file_module_with_filter(subdirectory, context, "__init__", file_filter)
+ {
// Check for a regular package first (highest priority).
- candidate.module = if is_legacy_namespace_package(&candidate.path, context, init) {
+ candidate.module = if is_legacy_namespace_package(subdirectory.path(), context, init) {
ResolvedModule::LegacyNamespacePackage(init)
} else {
ResolvedModule::RegularPackage(init)
};
- candidate.py_typed = candidate
- .path
+ candidate.py_typed = subdirectory
+ .path()
.py_typed(context)
.inherit_parent(candidate.py_typed);
- Ok(())
} else if let Some(file_module) =
- resolve_file_module_with_filter(&module_directory, context, module_name, file_filter)
+ resolve_file_module_with_filter(module_directory, context, module_name, file_filter)
{
// Check for a file module next
+ // A file module is terminal; keep its containing directory for its search-path origin.
candidate.module = ResolvedModule::Module(file_module);
- Ok(())
+ return Ok(());
} else {
// Last resort, check if a folder with the given name exists. If so,
// then this is a namespace package. We need to skip this check for
@@ -1748,22 +1759,27 @@
// `VERSIONS` file into consideration.
// A namespace package is not backed by a file, so it cannot satisfy a stub-only lookup.
if file_filter != ComponentFileFilter::StubOnly
- && !candidate.path.search_path().is_standard_library()
- && subdirectory.is_some()
+ && let Some(subdirectory) = &subdirectory
+ && !subdirectory.path().search_path().is_standard_library()
{
candidate.module = ResolvedModule::NamespacePackage;
- candidate.py_typed = candidate
- .path
+ candidate.py_typed = subdirectory
+ .path()
.py_typed(context)
.inherit_parent(candidate.py_typed);
- Ok(())
} else {
- Err(())
+ return Err(());
}
}
+
+ if let Some(subdirectory) = subdirectory {
+ candidate.directory = subdirectory;
+ }
+
+ Ok(())
}
-type ResolvedNames = Vec<ModuleResolutionCandidate>;
+type ResolvedNames<'db> = Vec<ModuleResolutionCandidate<'db>>;
/// If `module` exists on disk with an extension permitted by the resolver's mode, return its
/// [`File`].
