[ty] Include imported sub-modules as attributes on modules for completions (#18898)

This also adds a new `ModuleName::relative_to` public API to help with
this.

Kudos to @AlexWaygood for the meat of this patch!

Ref https://github.com/astral-sh/ruff/pull/18830#discussion_r2161770991
This commit is contained in:
Andrew Gallant 2025-06-23 12:48:16 -04:00 committed by GitHub
parent ef8281b695
commit d01e0faee3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 76 additions and 8 deletions

View file

@ -2148,9 +2148,7 @@ import importlib.resources
importlib.<CURSOR>
",
);
// TODO: This is wrong. Completions should include
// `resources` here.
test.assert_completions_do_not_include("resources");
test.assert_completions_include("resources");
}
#[test]
@ -2162,9 +2160,7 @@ import importlib.resources
importlib.<CURSOR>
",
);
// TODO: This is wrong. Completions should include
// `resources` here.
test.assert_completions_do_not_include("resources");
test.assert_completions_include("resources");
}
#[test]

View file

@ -127,6 +127,67 @@ impl ModuleName {
true
}
/// Given a parent module name of this module name, return the relative
/// portion of this module name.
///
/// For example, a parent module name of `importlib` with this module name
/// as `importlib.resources`, this returns `resources`.
///
/// If `parent` isn't a parent name of this module name, then this returns
/// `None`.
///
/// # Examples
///
/// This example shows some cases where `parent` is an actual parent of the
/// module name:
///
/// ```
/// use ty_python_semantic::ModuleName;
///
/// let this = ModuleName::new_static("importlib.resources").unwrap();
/// let parent = ModuleName::new_static("importlib").unwrap();
/// assert_eq!(this.relative_to(&parent), ModuleName::new_static("resources"));
///
/// let this = ModuleName::new_static("foo.bar.baz.quux").unwrap();
/// let parent = ModuleName::new_static("foo.bar").unwrap();
/// assert_eq!(this.relative_to(&parent), ModuleName::new_static("baz.quux"));
/// ```
///
/// This shows some cases where it isn't a parent:
///
/// ```
/// use ty_python_semantic::ModuleName;
///
/// let this = ModuleName::new_static("importliblib.resources").unwrap();
/// let parent = ModuleName::new_static("importlib").unwrap();
/// assert_eq!(this.relative_to(&parent), None);
///
/// let this = ModuleName::new_static("foo.bar.baz.quux").unwrap();
/// let parent = ModuleName::new_static("foo.barbaz").unwrap();
/// assert_eq!(this.relative_to(&parent), None);
///
/// let this = ModuleName::new_static("importlibbbbb.resources").unwrap();
/// let parent = ModuleName::new_static("importlib").unwrap();
/// assert_eq!(this.relative_to(&parent), None);
/// ```
#[must_use]
pub fn relative_to(&self, parent: &ModuleName) -> Option<ModuleName> {
let relative_name = self.0.strip_prefix(&*parent.0)?.strip_prefix('.')?;
// At this point, `relative_name` *has* to be a
// proper suffix of `self`. Otherwise, one of the two
// `strip_prefix` calls above would return `None`.
// (Notably, a valid `ModuleName` cannot end with a `.`.)
assert!(!relative_name.is_empty());
// This must also be true for this implementation to be
// correct. That is, the parent must be a prefix of this
// module name according to the rules of how module name
// components are split up. This could technically trip if
// the implementation of `starts_with` diverges from the
// implementation in this routine. But that seems unlikely.
debug_assert!(self.starts_with(parent));
Some(ModuleName(CompactString::from(relative_name)))
}
#[must_use]
#[inline]
pub fn as_str(&self) -> &str {

View file

@ -2,7 +2,7 @@ use crate::Db;
use crate::place::{imported_symbol, place_from_bindings, place_from_declarations};
use crate::semantic_index::place::ScopeId;
use crate::semantic_index::{
attribute_scopes, global_scope, place_table, semantic_index, use_def_map,
attribute_scopes, global_scope, imported_modules, place_table, semantic_index, use_def_map,
};
use crate::types::{ClassBase, ClassLiteral, KnownClass, Type};
use ruff_python_ast::name::Name;
@ -130,8 +130,9 @@ impl AllMembers {
Type::ModuleLiteral(literal) => {
self.extend_with_type(db, KnownClass::ModuleType.to_instance(db));
let module = literal.module(db);
let Some(file) = literal.module(db).file() else {
let Some(file) = module.file() else {
return;
};
@ -151,6 +152,16 @@ impl AllMembers {
.insert(place_table.place_expr(symbol_id).expect_name().clone());
}
}
let module_name = module.name();
self.members.extend(
imported_modules(db, literal.importing_file(db))
.iter()
.filter_map(|submodule_name| submodule_name.relative_to(module_name))
.filter_map(|relative_submodule_name| {
Some(Name::from(relative_submodule_name.components().next()?))
}),
);
}
}
}