Skip to content

[ty] Support renaming import aliases #21792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MichaReiser merged 7 commits into from
Dec 5, 2025
Merged

[ty] Support renaming import aliases #21792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b4e33cf
[ty] Support renaming import aliases
MichaReiser Dec 4, 2025
0b06f4b
Nits
MichaReiser Dec 4, 2025
179c0b4
Rename
MichaReiser Dec 5, 2025
4193b16
Fix more tests
MichaReiser Dec 5, 2025
9d04026
Comment
MichaReiser Dec 5, 2025
76568d0
Simplify
MichaReiser Dec 5, 2025
adf468b
Nits
MichaReiser Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 87 additions & 63 deletions crates/ty_ide/src/goto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,29 @@ pub(crate) enum GotoTarget<'a> {
/// ```
ImportModuleAlias {
alias: &'a ast::Alias,
asname: &'a ast::Identifier,
},

/// Import alias in from import statement
/// In an import statement, the named under which the symbol is exported
/// in the imported file.
///
/// ```py
/// from foo import bar as baz
/// ^^^
/// ```
ImportExportedName {
alias: &'a ast::Alias,
import_from: &'a ast::StmtImportFrom,
},

/// Import alias in from import statement
/// ```py
/// from foo import bar as baz
/// ^^^
/// ```
ImportSymbolAlias {
alias: &'a ast::Alias,
range: TextRange,
import_from: &'a ast::StmtImportFrom,
asname: &'a ast::Identifier,
},

/// Go to on the exception handler variable
Expand Down Expand Up @@ -290,8 +300,9 @@ impl GotoTarget<'_> {
GotoTarget::FunctionDef(function) => function.inferred_type(model),
GotoTarget::ClassDef(class) => class.inferred_type(model),
GotoTarget::Parameter(parameter) => parameter.inferred_type(model),
GotoTarget::ImportSymbolAlias { alias, .. } => alias.inferred_type(model),
GotoTarget::ImportModuleAlias { alias } => alias.inferred_type(model),
GotoTarget::ImportSymbolAlias { alias, .. }
| GotoTarget::ImportModuleAlias { alias, .. }
| GotoTarget::ImportExportedName { alias, .. } => alias.inferred_type(model),
GotoTarget::ExceptVariable(except) => except.inferred_type(model),
GotoTarget::KeywordArgument { keyword, .. } => keyword.value.inferred_type(model),
// When asking the type of a callable, usually you want the callable itself?
Expand Down Expand Up @@ -378,7 +389,9 @@ impl GotoTarget<'_> {
alias_resolution: ImportAliasResolution,
) -> Option<Definitions<'db>> {
let definitions = match self {
GotoTarget::Expression(expression) => definitions_for_expression(model, *expression),
GotoTarget::Expression(expression) => {
definitions_for_expression(model, *expression, alias_resolution)
}
// For already-defined symbols, they are their own definitions
GotoTarget::FunctionDef(function) => Some(vec![ResolvedDefinition::Definition(
function.definition(model),
Expand All @@ -393,22 +406,21 @@ impl GotoTarget<'_> {
)]),

// For import aliases (offset within 'y' or 'z' in "from x import y as z")
GotoTarget::ImportSymbolAlias {
alias, import_from, ..
} => {
if let Some(asname) = alias.asname.as_ref()
&& alias_resolution == ImportAliasResolution::PreserveAliases
{
Some(definitions_for_name(model, asname.as_str(), asname.into()))
} else {
let symbol_name = alias.name.as_str();
Some(definitions_for_imported_symbol(
model,
import_from,
symbol_name,
alias_resolution,
))
}
GotoTarget::ImportSymbolAlias { asname, .. } => Some(definitions_for_name(
model,
asname.as_str(),
AnyNodeRef::from(*asname),
alias_resolution,
)),

GotoTarget::ImportExportedName { alias, import_from } => {
let symbol_name = alias.name.as_str();
Some(definitions_for_imported_symbol(
model,
import_from,
symbol_name,
alias_resolution,
))
}

GotoTarget::ImportModuleComponent {
Expand All @@ -423,15 +435,12 @@ impl GotoTarget<'_> {
}

// Handle import aliases (offset within 'z' in "import x.y as z")
GotoTarget::ImportModuleAlias { alias } => {
if let Some(asname) = alias.asname.as_ref()
&& alias_resolution == ImportAliasResolution::PreserveAliases
{
Some(definitions_for_name(model, asname.as_str(), asname.into()))
} else {
definitions_for_module(model, Some(alias.name.as_str()), 0)
}
}
GotoTarget::ImportModuleAlias { asname, .. } => Some(definitions_for_name(
model,
asname.as_str(),
AnyNodeRef::from(*asname),
alias_resolution,
)),

// Handle keyword arguments in call expressions
GotoTarget::KeywordArgument {
Expand All @@ -454,12 +463,22 @@ impl GotoTarget<'_> {
// because they're not expressions
GotoTarget::PatternMatchRest(pattern_mapping) => {
pattern_mapping.rest.as_ref().map(|name| {
definitions_for_name(model, name.as_str(), AnyNodeRef::Identifier(name))
definitions_for_name(
model,
name.as_str(),
AnyNodeRef::Identifier(name),
alias_resolution,
)
})
}

GotoTarget::PatternMatchAsName(pattern_as) => pattern_as.name.as_ref().map(|name| {
definitions_for_name(model, name.as_str(), AnyNodeRef::Identifier(name))
definitions_for_name(
model,
name.as_str(),
AnyNodeRef::Identifier(name),
alias_resolution,
)
}),

GotoTarget::PatternKeywordArgument(pattern_keyword) => {
Expand All @@ -468,22 +487,37 @@ impl GotoTarget<'_> {
model,
name.as_str(),
AnyNodeRef::Identifier(name),
alias_resolution,
))
}

GotoTarget::PatternMatchStarName(pattern_star) => {
pattern_star.name.as_ref().map(|name| {
definitions_for_name(model, name.as_str(), AnyNodeRef::Identifier(name))
definitions_for_name(
model,
name.as_str(),
AnyNodeRef::Identifier(name),
alias_resolution,
)
})
}

// For callables, both the definition of the callable and the actual function impl are relevant.
//
// Prefer the function impl over the callable so that its docstrings win if defined.
GotoTarget::Call { callable, call } => {
let mut definitions = definitions_for_callable(model, call);
let mut definitions = Vec::new();

// We prefer the specific overload for hover, go-to-def etc. However,
// `definitions_for_callable` always resolves import aliases. That's why we
// skip it in cases import alias resolution is turned of (rename, highlight references).
if alias_resolution == ImportAliasResolution::ResolveAliases {
definitions.extend(definitions_for_callable(model, call));
}
Comment on lines +511 to +516
Copy link
Contributor

@Gankra Gankra Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concretely: the reason GotoTarget::Call exists if for when you click on Class(). definitions_for_expression will find class Class while definitions_for_callable will find def __init__(). So indeed we want to excludedefinitions_for_callable when we don't want to ResolveAliases!

Copy link
Member Author

@MichaReiser MichaReiser Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, thank you! That makes sense


let expr_definitions =
definitions_for_expression(model, *callable).unwrap_or_default();
definitions_for_expression(model, *callable, alias_resolution)
.unwrap_or_default();
definitions.extend(expr_definitions);

if definitions.is_empty() {
Expand Down Expand Up @@ -517,7 +551,7 @@ impl GotoTarget<'_> {
let subexpr = covering_node(subast.syntax().into(), *subrange)
.node()
.as_expr_ref()?;
definitions_for_expression(&submodel, subexpr)
definitions_for_expression(&submodel, subexpr, alias_resolution)
}

// nonlocal and global are essentially loads, but again they're statements,
Expand All @@ -527,6 +561,7 @@ impl GotoTarget<'_> {
model,
identifier.as_str(),
AnyNodeRef::Identifier(identifier),
alias_resolution,
))
}

Expand All @@ -537,6 +572,7 @@ impl GotoTarget<'_> {
model,
name.as_str(),
AnyNodeRef::Identifier(name),
alias_resolution,
))
}

Expand All @@ -546,6 +582,7 @@ impl GotoTarget<'_> {
model,
name.as_str(),
AnyNodeRef::Identifier(name),
alias_resolution,
))
}

Expand All @@ -555,6 +592,7 @@ impl GotoTarget<'_> {
model,
name.as_str(),
AnyNodeRef::Identifier(name),
alias_resolution,
))
}
};
Expand All @@ -580,12 +618,9 @@ impl GotoTarget<'_> {
GotoTarget::FunctionDef(function) => Some(Cow::Borrowed(function.name.as_str())),
GotoTarget::ClassDef(class) => Some(Cow::Borrowed(class.name.as_str())),
GotoTarget::Parameter(parameter) => Some(Cow::Borrowed(parameter.name.as_str())),
GotoTarget::ImportSymbolAlias { alias, .. } => {
if let Some(asname) = &alias.asname {
Some(Cow::Borrowed(asname.as_str()))
} else {
Some(Cow::Borrowed(alias.name.as_str()))
}
GotoTarget::ImportSymbolAlias { asname, .. } => Some(Cow::Borrowed(asname.as_str())),
GotoTarget::ImportExportedName { alias, .. } => {
Some(Cow::Borrowed(alias.name.as_str()))
}
GotoTarget::ImportModuleComponent {
module_name,
Expand All @@ -599,13 +634,7 @@ impl GotoTarget<'_> {
Some(Cow::Borrowed(module_name))
}
}
GotoTarget::ImportModuleAlias { alias } => {
if let Some(asname) = &alias.asname {
Some(Cow::Borrowed(asname.as_str()))
} else {
Some(Cow::Borrowed(alias.name.as_str()))
}
}
GotoTarget::ImportModuleAlias { asname, .. } => Some(Cow::Borrowed(asname.as_str())),
GotoTarget::ExceptVariable(except) => {
Some(Cow::Borrowed(except.name.as_ref()?.as_str()))
}
Expand Down Expand Up @@ -667,7 +696,7 @@ impl GotoTarget<'_> {
// Is the offset within the alias name (asname) part?
if let Some(asname) = &alias.asname {
if asname.range.contains_inclusive(offset) {
return Some(GotoTarget::ImportModuleAlias { alias });
return Some(GotoTarget::ImportModuleAlias { alias, asname });
}
}

Expand Down Expand Up @@ -699,21 +728,13 @@ impl GotoTarget<'_> {
// Is the offset within the alias name (asname) part?
if let Some(asname) = &alias.asname {
if asname.range.contains_inclusive(offset) {
return Some(GotoTarget::ImportSymbolAlias {
alias,
range: asname.range,
import_from,
});
return Some(GotoTarget::ImportSymbolAlias { alias, asname });
}
}

// Is the offset in the original name part?
if alias.name.range.contains_inclusive(offset) {
return Some(GotoTarget::ImportSymbolAlias {
alias,
range: alias.name.range,
import_from,
});
return Some(GotoTarget::ImportExportedName { alias, import_from });
}

None
Expand Down Expand Up @@ -893,12 +914,13 @@ impl Ranged for GotoTarget<'_> {
GotoTarget::FunctionDef(function) => function.name.range,
GotoTarget::ClassDef(class) => class.name.range,
GotoTarget::Parameter(parameter) => parameter.name.range,
GotoTarget::ImportSymbolAlias { range, .. } => *range,
GotoTarget::ImportSymbolAlias { asname, .. } => asname.range,
Self::ImportExportedName { alias, .. } => alias.name.range,
GotoTarget::ImportModuleComponent {
component_range, ..
} => *component_range,
GotoTarget::StringAnnotationSubexpr { subrange, .. } => *subrange,
GotoTarget::ImportModuleAlias { alias } => alias.asname.as_ref().unwrap().range,
GotoTarget::ImportModuleAlias { asname, .. } => asname.range,
GotoTarget::ExceptVariable(except) => except.name.as_ref().unwrap().range,
GotoTarget::KeywordArgument { keyword, .. } => keyword.arg.as_ref().unwrap().range,
GotoTarget::PatternMatchRest(rest) => rest.rest.as_ref().unwrap().range,
Expand Down Expand Up @@ -955,12 +977,14 @@ fn convert_resolved_definitions_to_targets<'db>(
fn definitions_for_expression<'db>(
model: &SemanticModel<'db>,
expression: ruff_python_ast::ExprRef<'_>,
alias_resolution: ImportAliasResolution,
) -> Option<Vec<ResolvedDefinition<'db>>> {
match expression {
ast::ExprRef::Name(name) => Some(definitions_for_name(
model,
name.id.as_str(),
expression.into(),
alias_resolution,
)),
ast::ExprRef::Attribute(attribute) => Some(ty_python_semantic::definitions_for_attribute(
model, attribute,
Expand Down
39 changes: 34 additions & 5 deletions crates/ty_ide/src/references.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,38 @@ pub enum ReferencesMode {
DocumentHighlights,
}

impl ReferencesMode {
Copy link
Member Author

@MichaReiser MichaReiser Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to unify ReferencesMode and ImportAliasResolution because I think there might be more differences in resolve_* moving forward where it might even be beneficial to know: Hey, what request are we handling, it's rename, cool, let's do this instead of x.

pub(super) fn to_import_alias_resolution(self) -> ImportAliasResolution {
match self {
// Resolve import aliases for find references:
// ```py
// from warnings import deprecated as my_deprecated
//
// @my_deprecated
// def foo
// ```
//
// When finding references on `my_deprecated`, we want to find all usages of `deprecated` across the entire
// project.
Self::References | Self::ReferencesSkipDeclaration => {
ImportAliasResolution::ResolveAliases
}
// For rename, don't resolve import aliases.
//
// ```py
// from warnings import deprecated as my_deprecated
//
// @my_deprecated
// def foo
// ```
// When renaming `my_deprecated`, only rename the alias, but not the original definition in `warnings`.
Self::Rename | Self::RenameMultiFile | Self::DocumentHighlights => {
ImportAliasResolution::PreserveAliases
}
}
}
}

/// Find all references to a symbol at the given position.
/// Search for references across all files in the project.
pub(crate) fn references(
Expand All @@ -45,12 +77,9 @@ pub(crate) fn references(
goto_target: &GotoTarget,
mode: ReferencesMode,
) -> Option<Vec<ReferenceTarget>> {
// Get the definitions for the symbol at the cursor position

// When finding references, do not resolve any local aliases.
let model = SemanticModel::new(db, file);
let target_definitions = goto_target
.get_definition_targets(&model, ImportAliasResolution::PreserveAliases)?
.get_definition_targets(&model, mode.to_import_alias_resolution())?
.declaration_targets(db)?;

// Extract the target text from the goto target for fast comparison
Expand Down Expand Up @@ -318,7 +347,7 @@ impl LocalReferencesFinder<'_> {
{
// Get the definitions for this goto target
if let Some(current_definitions) = goto_target
.get_definition_targets(self.model, ImportAliasResolution::PreserveAliases)
.get_definition_targets(self.model, self.mode.to_import_alias_resolution())
.and_then(|definitions| definitions.declaration_targets(self.model.db()))
{
// Check if any of the current definitions match our target definitions
Expand Down
Loading
Loading