Skip to content

Commit ce79443

Browse files
authored
[ty] Return root discovery to NameResolver, but retain necessary interface changes (#28536)
<!-- 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 <!-- What's the purpose of the change? What does it do, and why? --> #28531 incorrectly went a bit too far in turning `NameResolver::discover_roots` into a free function. That isn't actually necessary for the refactor we're aiming for; we just need to widen its interface to accept new arguments that allows it to apply to new contexts. This restores the private method but retains the wider interface. ## Test Plan This is a behaviour-preserving refactor that relies on existing tests. <!-- How was it tested? -->
1 parent e11135d commit ce79443

1 file changed

Lines changed: 68 additions & 71 deletions

File tree

‎crates/ty_module_resolver/src/resolve.rs‎

Lines changed: 68 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,8 +1435,7 @@ impl<'db, 'name> NameResolver<'db, 'name> {
14351435
/// packages as parents, but its final module must come from a stub file.
14361436
fn resolve_typing(&self, stub_packages: &StubPackageIndex) -> Option<ResolvedNames<'db>> {
14371437
if self.name.components().nth(1).is_none() {
1438-
let candidates = discover_roots(
1439-
&self.context,
1438+
let candidates = self.discover_roots(
14401439
self.name.first_component(),
14411440
self.is_non_shadowable,
14421441
search_paths(
@@ -1453,8 +1452,7 @@ impl<'db, 'name> NameResolver<'db, 'name> {
14531452
// be shadowed before the resolver reaches the requested stub. Reuse those roots for the
14541453
// normal fallback so that each extra path is probed only once.
14551454
let (overlay_stub_packages, remaining_stub_packages) = stub_packages.split_overlay();
1456-
let mut candidates = discover_roots(
1457-
&self.context,
1455+
let mut candidates = self.discover_roots(
14581456
self.name.first_component(),
14591457
self.is_non_shadowable,
14601458
search_paths(
@@ -1471,8 +1469,7 @@ impl<'db, 'name> NameResolver<'db, 'name> {
14711469
return Some(resolved);
14721470
}
14731471

1474-
let remaining_candidates = discover_roots(
1475-
&self.context,
1472+
let remaining_candidates = self.discover_roots(
14761473
self.name.first_component(),
14771474
self.is_non_shadowable,
14781475
search_paths(
@@ -1496,8 +1493,7 @@ impl<'db, 'name> NameResolver<'db, 'name> {
14961493
fn resolve_desperate_typing(&self, search_paths: &[SearchPath]) -> Option<ResolvedNames<'db>> {
14971494
let stub_packages =
14981495
StubPackageIndex::from_search_paths(self.context.db, search_paths.iter());
1499-
let candidates = discover_roots(
1500-
&self.context,
1496+
let candidates = self.discover_roots(
15011497
self.name.first_component(),
15021498
self.is_non_shadowable,
15031499
search_paths.iter(),
@@ -1514,8 +1510,7 @@ impl<'db, 'name> NameResolver<'db, 'name> {
15141510
&self,
15151511
search_paths: impl Iterator<Item = &'a SearchPath>,
15161512
) -> Option<ResolvedNames<'db>> {
1517-
let candidates = discover_roots(
1518-
&self.context,
1513+
let candidates = self.discover_roots(
15191514
self.name.first_component(),
15201515
self.is_non_shadowable,
15211516
search_paths,
@@ -1573,73 +1568,75 @@ impl<'db, 'name> NameResolver<'db, 'name> {
15731568
}
15741569
}
15751570
}
1576-
}
15771571

1578-
/// Finds candidates for a top-level name across the supplied search paths and stub packages.
1579-
fn discover_roots<'a, 'db>(
1580-
context: &ResolverContext<'db>,
1581-
root_component: &str,
1582-
is_non_shadowable: bool,
1583-
search_paths: impl Iterator<Item = &'a SearchPath>,
1584-
stub_paths: StubPackagePaths<'_>,
1585-
) -> ResolvedNames<'db> {
1586-
let mut cur_candidates = Vec::new();
1587-
let stub_name =
1588-
(!stub_paths.is_empty() && !is_non_shadowable).then(|| format!("{root_component}-stubs"));
1589-
let mut pending_stub_paths = Vec::new();
1590-
1591-
if let Some(stub_name) = &stub_name {
1592-
cur_candidates.extend(stub_paths.before_stdlib.iter().filter_map(|search_path| {
1593-
resolve_stub_package_in_search_path(context, search_path, stub_name)
1594-
}));
1595-
// Defer file probes after stdlib until we know that stdlib does not win.
1596-
pending_stub_paths.extend(stub_paths.after_stdlib.iter().filter(|search_path| {
1597-
ModuleDirectory::new(context, search_path.to_module_path()).may_contain_name(stub_name)
1598-
}));
1599-
}
1600-
1601-
for search_path in search_paths {
1602-
// When a builtin module is imported, standard module resolution is bypassed:
1603-
// the module name always resolves to the stdlib module,
1604-
// even if there's a module of the same name in the first-party root
1605-
// (which would normally result in the stdlib module being overridden).
1606-
// TODO: offer a diagnostic if there is a first-party module of the same name
1607-
if is_non_shadowable && !search_path.is_standard_library() {
1608-
continue;
1572+
/// Finds candidates for a top-level name across the supplied search paths and stub packages.
1573+
fn discover_roots<'a>(
1574+
&self,
1575+
root_component: &str,
1576+
is_non_shadowable: bool,
1577+
search_paths: impl Iterator<Item = &'a SearchPath>,
1578+
stub_paths: StubPackagePaths<'_>,
1579+
) -> ResolvedNames<'db> {
1580+
let context = &self.context;
1581+
let mut cur_candidates = Vec::new();
1582+
let stub_name = (!stub_paths.is_empty() && !is_non_shadowable)
1583+
.then(|| format!("{root_component}-stubs"));
1584+
let mut pending_stub_paths = Vec::new();
1585+
1586+
if let Some(stub_name) = &stub_name {
1587+
cur_candidates.extend(stub_paths.before_stdlib.iter().filter_map(|search_path| {
1588+
resolve_stub_package_in_search_path(context, search_path, stub_name)
1589+
}));
1590+
// Defer file probes after stdlib until we know that stdlib does not win.
1591+
pending_stub_paths.extend(stub_paths.after_stdlib.iter().filter(|search_path| {
1592+
ModuleDirectory::new(context, search_path.to_module_path())
1593+
.may_contain_name(stub_name)
1594+
}));
16091595
}
16101596

1611-
let is_stdlib = search_path.is_standard_library();
1612-
// A terminal candidate can stop the search unless a matching post-stdlib stub package
1613-
// could still override it. A terminal stdlib candidate always stops the search.
1614-
let can_stop = is_stdlib || pending_stub_paths.is_empty();
1615-
let mut candidate = ModuleResolutionCandidate::root(context, search_path);
1616-
let resolved = resolve_component(
1617-
context,
1618-
&mut candidate,
1619-
root_component,
1620-
ComponentFileFilter::ByMode,
1621-
)
1622-
.is_ok();
1623-
let terminal = candidate.missing_submodule_is_terminal();
1624-
if resolved {
1625-
cur_candidates.push(candidate);
1626-
}
1627-
// A terminal candidate shadows all later search paths. Earlier candidates remain in
1628-
// play because they already shadow this candidate.
1629-
if terminal && can_stop {
1630-
break;
1631-
}
1597+
for search_path in search_paths {
1598+
// When a builtin module is imported, standard module resolution is bypassed:
1599+
// the module name always resolves to the stdlib module,
1600+
// even if there's a module of the same name in the first-party root
1601+
// (which would normally result in the stdlib module being overridden).
1602+
// TODO: offer a diagnostic if there is a first-party module of the same name
1603+
if is_non_shadowable && !search_path.is_standard_library() {
1604+
continue;
1605+
}
16321606

1633-
// Reaching this point for stdlib means that it did not provide a terminal candidate.
1634-
// The deferred post-stdlib stub packages are therefore eligible, so resolve them now.
1635-
if is_stdlib && let Some(stub_name) = &stub_name {
1636-
cur_candidates.extend(pending_stub_paths.drain(..).filter_map(|search_path| {
1637-
resolve_stub_package_in_search_path(context, search_path, stub_name)
1638-
}));
1607+
let is_stdlib = search_path.is_standard_library();
1608+
// A terminal candidate can stop the search unless a matching post-stdlib stub package
1609+
// could still override it. A terminal stdlib candidate always stops the search.
1610+
let can_stop = is_stdlib || pending_stub_paths.is_empty();
1611+
let mut candidate = ModuleResolutionCandidate::root(context, search_path);
1612+
let resolved = resolve_component(
1613+
context,
1614+
&mut candidate,
1615+
root_component,
1616+
ComponentFileFilter::ByMode,
1617+
)
1618+
.is_ok();
1619+
let terminal = candidate.missing_submodule_is_terminal();
1620+
if resolved {
1621+
cur_candidates.push(candidate);
1622+
}
1623+
// A terminal candidate shadows all later search paths. Earlier candidates remain in
1624+
// play because they already shadow this candidate.
1625+
if terminal && can_stop {
1626+
break;
1627+
}
1628+
1629+
// Reaching this point for stdlib means that it did not provide a terminal candidate.
1630+
// The deferred post-stdlib stub packages are therefore eligible, so resolve them now.
1631+
if is_stdlib && let Some(stub_name) = &stub_name {
1632+
cur_candidates.extend(pending_stub_paths.drain(..).filter_map(|search_path| {
1633+
resolve_stub_package_in_search_path(context, search_path, stub_name)
1634+
}));
1635+
}
16391636
}
1640-
}
16411637

1642-
cur_candidates
1638+
cur_candidates
1639+
}
16431640
}
16441641

16451642
fn resolve_stub_package_in_search_path<'db>(

0 commit comments

Comments
 (0)