[ty] Support renaming import aliases by MichaReiser · Pull Request #21792 · astral-sh/ruff

MichaReiser · 2025-12-04T14:26:16Z

Summary

Fixes renaming import aliases or symbols that include an import alias in their chain.

For renames (and highlight references), we mustn't resolve to the final declaration. Instead, we should only resolve to the first declaration that defined this specific name (e.g. the import alias). We already made this distinction internally but we didn't correctly propagate the alias_resolution, which resulted in some definitions resolving to the final declarations and others only resolved to the first import alias.

It's fairly likely that we still fail to propagate ImportAliasResolution in some places but we can fix those as they come up.

This PR fixes this.

I also decided to split GotoTarget::ImportSymbolAlias into two variants, one for when the target is the alias's asname and one where it's the alias's name. Mainly because I found the distinction only based on range hard to follow and this also revealed some further bugs.

Fixes astral-sh/ty#1661

Test plan

Screen.Recording.2025-12-05.at.11.54.26.mov

astral-sh-bot · 2025-12-04T14:28:15Z

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

astral-sh-bot · 2025-12-04T14:30:13Z

`mypy_primer` results

Changes were detected when running on open source projects

beartype (https://github.com/beartype/beartype)
- beartype/claw/_package/clawpkgtrie.py:66:29: warning[unsupported-base] Unsupported class base with type `<class 'dict[str, PackagesTrieBlacklist]'> | <class 'dict[str, Divergent]'>`
- beartype/claw/_package/clawpkgtrie.py:247:29: warning[unsupported-base] Unsupported class base with type `<class 'dict[str, PackagesTrieWhitelist]'> | <class 'dict[str, Divergent]'>`
- Found 494 diagnostics
+ Found 492 diagnostics

scikit-build-core (https://github.com/scikit-build/scikit-build-core)
- src/scikit_build_core/_logging.py:153:13: warning[unsupported-base] Unsupported class base with type `<class 'Mapping[str, Style]'> | <class 'Mapping[str, Divergent]'>`
- src/scikit_build_core/build/_pathutil.py:25:38: error[invalid-argument-type] Argument to function `__new__` is incorrect: Expected `str | PathLike[str]`, found `DirEntry[Path]`
- src/scikit_build_core/build/_pathutil.py:27:24: error[invalid-argument-type] Argument to function `__new__` is incorrect: Expected `str | PathLike[str]`, found `DirEntry[Path]`
- src/scikit_build_core/build/wheel.py:98:20: error[no-matching-overload] No overload of bound method `__init__` matches arguments
- Found 42 diagnostics
+ Found 38 diagnostics

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
- pandas-stubs/_typing.pyi:1217:16: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- Found 5518 diagnostics
+ Found 5517 diagnostics

No memory usage changes detected ✅

crates/ty_ide/src/goto.rs

crates/ty_ide/src/rename.rs

codspeed-hq · 2025-12-04T18:22:32Z

CodSpeed Performance Report

Merging #21792 will not alter performance

_{Comparing micha/rename-import-alias (adf468b) with main (b2fb421)}

Summary

✅ 22 untouched
⏩ 30 skipped¹

30 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

MichaReiser · 2025-12-05T13:54:56Z

crates/ty_ide/src/references.rs

    DocumentHighlights,
 }

+impl ReferencesMode {


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.

Gankra

Hmm is this rebased on main? I think some from..import tests I added should also have had snapshot updates.

Gankra · 2025-12-05T15:16:06Z

crates/ty_ide/src/goto.rs

+                // 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));
+                }


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!

Ohh, thank you! That makes sense

crates/ty_ide/src/rename.rs

Gankra · 2025-12-05T15:26:55Z

crates/ty_ide/src/rename.rs

+            .source(
+                "main.py",
+                r#"
+                from lib import deprecated<CURSOR>


the absolute confidence of a ruff dev to be like "yeah of course that cursor is on deprecated"

I debugged way too many TextRange problems :)

MichaReiser · 2025-12-05T18:02:40Z

Hmm is this rebased on main? I think some from..import tests I added should also have had snapshot updates.

I rebased a couple of times today. Any specific examples? I don't see any other rename tests that use an alias.

I did see some changes to your submodule import tests when trying to fix overload resolution that looked related to redeclarations.

MichaReiser · 2025-12-05T18:06:13Z

I'll go ahead and merge this. Happy to address any tests that should have been updated but didn't in a follow up PR

Gankra · 2025-12-05T18:23:30Z

Ah good point, the cases I got in are specifically failing on non-aliases, so no worries.

* origin/main: [ty] Allow `tuple[Any, ...]` to assign to `tuple[int, *tuple[int, ...]]` (#21803) [ty] Support renaming import aliases (#21792) [ty] Add redeclaration LSP tests (#21812) [ty] more detailed description of "Size limit on unions of literals" in mdtest (#21804) [ty] Complete support for `ParamSpec` (#21445) [ty] Update benchmark dependencies (#21815)

* origin/main: [ty] Add test case for fixed panic (#21832) [ty] Avoid double-analyzing tuple in `Final` subscript (#21828) [flake8-bandit] Fix false positive when using non-standard `CSafeLoader` path (S506). (#21830) Add minimal-size build profile (#21826) [ty] Allow `tuple[Any, ...]` to assign to `tuple[int, *tuple[int, ...]]` (#21803) [ty] Support renaming import aliases (#21792) [ty] Add redeclaration LSP tests (#21812) [ty] more detailed description of "Size limit on unions of literals" in mdtest (#21804) [ty] Complete support for `ParamSpec` (#21445) [ty] Update benchmark dependencies (#21815)

MichaReiser added server Related to the LSP server ty Multi-file analysis & type inference labels Dec 4, 2025

MichaReiser commented Dec 4, 2025

View reviewed changes

crates/ty_ide/src/goto.rs Outdated Show resolved Hide resolved

Gankra reviewed Dec 4, 2025

View reviewed changes

MichaReiser commented Dec 4, 2025

View reviewed changes

crates/ty_ide/src/rename.rs Outdated Show resolved Hide resolved

MichaReiser force-pushed the micha/rename-import-alias branch from 89e5f3e to 86b8077 Compare December 5, 2025 10:41

MichaReiser marked this pull request as ready for review December 5, 2025 10:54

MichaReiser requested review from AlexWaygood, carljm, dcreager and sharkdp as code owners December 5, 2025 10:54

MichaReiser removed request for AlexWaygood, dcreager and sharkdp December 5, 2025 10:54

MichaReiser force-pushed the micha/rename-import-alias branch from 8d919bb to 56408e3 Compare December 5, 2025 13:53

MichaReiser commented Dec 5, 2025

View reviewed changes

Gankra approved these changes Dec 5, 2025

View reviewed changes

MichaReiser added 7 commits December 5, 2025 19:03

[ty] Support renaming import aliases

b4e33cf

Nits

0b06f4b

Rename

179c0b4

Fix more tests

4193b16

Comment

9d04026

Simplify

76568d0

Nits

adf468b

MichaReiser force-pushed the micha/rename-import-alias branch from 56408e3 to adf468b Compare December 5, 2025 18:04

MichaReiser merged commit 9714c58 into main Dec 5, 2025
40 checks passed

MichaReiser deleted the micha/rename-import-alias branch December 5, 2025 18:12

Conversation

MichaReiser commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

astral-sh-bot bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

astral-sh-bot bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codspeed-hq bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #21792 will not alter performance

Summary

Footnotes

Uh oh!

MichaReiser Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Gankra left a comment

Choose a reason for hiding this comment

Uh oh!

Gankra Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Gankra Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser commented Dec 5, 2025

Uh oh!

MichaReiser commented Dec 5, 2025

Uh oh!

Uh oh!

Gankra commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MichaReiser commented Dec 4, 2025 •

edited

Loading

astral-sh-bot bot commented Dec 4, 2025 •

edited

Loading

astral-sh-bot bot commented Dec 4, 2025 •

edited

Loading

`mypy_primer` results

codspeed-hq bot commented Dec 4, 2025 •

edited

Loading

MichaReiser Dec 5, 2025 •

edited

Loading