Skip to content

Attribute Python import names with ty's canonical types - #8408

Merged
knutwannheden merged 2 commits into
mainfrom
canonical-type-attribution-for-python-import-names
Aug 6, 2026
Merged

Attribute Python import names with ty's canonical types#8408
knutwannheden merged 2 commits into
mainfrom
canonical-type-attribution-for-python-import-names

Conversation

@knutwannheden

@knutwannheden knutwannheden commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Motivation

ty attributes a re-exported symbol at its definition site, not at the path a file imports it through. Usages of from os.path import join are attributed as posixpath.join, and from collections.abc import Iterable as typing.Iterable. Import qualids, meanwhile, carried no type at all, so nothing connected the two spellings: an FQN derived from type attribution never matched the import statement's written path, and a recipe asking RemoveImport to drop posixpath.join left the os.path import sitting there unused. The same gap runs the other way — a recipe pattern written against the public path could not be compared to what attribution reports.

The fix is entirely parser-side. ty's getTypes response already carries what is needed: for each import name it emits an Alias node at that name's byte range whose descriptor names the canonical location, e.g. {"nodeKind": "Alias", "start": 20, "end": 24, "typeId": 1} resolving to {"kind": "function", "moduleName": "posixpath", "name": "join"}. The parser now consumes those nodes, and the Python import machinery matches a requested (module, name) against both the written path and the canonical FQN.

Examples

Import names carry the canonical type of the symbol they bind:

cu = ParserVisitor(src, path, ty_client).visit_Module(ast.parse("from os.path import join\n"))
t = cu.statements[0].names[0].qualid.type
# JavaType.Method: name 'join', declaring type 'posixpath'

RemoveImport accepts either spelling, so a Java-side recipe that only knows the canonical FQN can clean up after itself:

# both of these remove `from os.path import join`
maybe_remove_import(self, RemoveImportOptions(module='os.path', name='join'))
maybe_remove_import(self, RemoveImportOptions(module='posixpath', name='join'))

AddImport treats an existing public-path import as already satisfying a canonical request, rather than adding a redundant second import:

# no-op when the file already has `from os.path import join`
maybe_add_import(self, AddImportOptions(module='posixpath', name='join'))

Summary

  • PythonTypeMapping.import_alias_type(ast.alias) maps ty's Alias descriptor to a JavaType: module kinds become the module's class, function kinds a whole JavaType.Method (not the return type that expression positions use, so callers can derive an FQN from declaring type plus name), and everything else resolves normally.
  • ParserVisitor.visit_alias attaches that type to the import's qualid and to any as alias identifier. For dotted names only the outermost FieldAccess — the one naming the full path — carries it.
  • A non-aliased dotted import os.path is typed by ty as the bound root package os; the qualid keeps the full dotted path, since that is what the name denotes.
  • New import_utils.get_canonical_fqn(imp) reads the canonical FQN back off an attributed import, joining declaring type and name for methods.
  • RemoveImport matches a member when the written module and name match or its canonical FQN equals the requested module.name. Whole-module removal additionally matches an import that binds the module itself (from os import path for os.path). Membership deliberately does not count: a module is the canonical home of every symbol re-exported through it, so matching members would sweep up imports written against other modules — RemoveImport(module='typing') would reach from collections.abc import Iterable.
  • AddImport's duplicate check accepts a canonically matching member, guarded on it binding the same name so references to the requested name still resolve. Merging a new name into an existing from statement stays purely syntactic — merging a canonical request into a differently-written statement would need a canonical-to-public reverse mapping, which is unbounded.

Test plan

  • tests/python/test_type_attribution.py::TestImportNameAttribution — 8 new cases covering re-exported functions (with and without an alias), re-exported classes, plain and dotted module imports, from X import <module>, and an unresolvable module staying untyped.
  • tests/python/test_remove_import.py::TestCanonicalRemoveImport — 6 new cases: removal by canonical FQN for functions and classes, canonical removal leaving sibling names intact, whole-module removal by module binding, a negative case (os.path.exists is canonically genericpath.exists, so a posixpath.exists request must not match), and a whole-module request sparing canonically-related members of another module.
  • tests/recipes/test_change_import.py::TestChangeImportLeavesUnrelatedImports — ChangeImport's whole-module removal of the old module leaves an unrelated, canonically-related import in place.
  • tests/python/test_add_import.py::TestCanonicalAddImportDedup — 4 new cases: canonical requests deduplicating against written from-imports for functions and classes, an aliased binding not satisfying a plain-name request, and a canonical mismatch still adding its own import.
  • Full non-RPC rewrite-python suite green: 1863 passed, 7 skipped.

ty attributes a re-exported symbol at its definition site, so a name imported
through its public path was unreachable from type attribution: usages of
`from os.path import join` are attributed as `posixpath.join`, and
`from collections.abc import Iterable` as `typing.Iterable`. Import qualids
carried no type at all, so nothing bridged the two spellings and a recipe
asking to remove `posixpath.join` left the `os.path` import in place.

The ty `getTypes` response already carries this: for each import name it emits
an `Alias` node at the name's byte range whose descriptor names the canonical
location. The parser now consumes it and attributes each import name, and the
import machinery matches a requested (module, name) against both the written
path and the canonical FQN.
A module is the canonical home of every symbol re-exported through it, so
testing a member's canonical parent against the requested module made
`RemoveImport(module='typing')` delete `from collections.abc import Iterable`.
Whole-module removal now matches only an import that binds the module itself,
which still covers `from os import path` for `os.path`. Name-level matching is
unaffected: it compares an exact `module.name` FQN.

The one in-tree caller is ChangeImport, which schedules a whole-module removal
for the old module after rewriting qualified references; a regression test
covers an unrelated import surviving that path.
@knutwannheden
knutwannheden merged commit 0f17b3c into main Aug 6, 2026
1 check passed
@knutwannheden
knutwannheden deleted the canonical-type-attribution-for-python-import-names branch August 6, 2026 10:22
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant