Skip to content
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.

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

pep585 import aliases #983

Closed
wants to merge 2 commits into from

Conversation

stephenfin
Copy link

Track aliases so that pep585 rewriting works with e.g.

import typing
x: ty.List[str] = []

Signed-off-by: Stephen Finucane <[email protected]>
State.as_import probably has uses elsewhere, but this is sufficient to
start with.

We need to update some of the pep604 feature tests to ensure we actually
do the import.

Signed-off-by: Stephen Finucane <[email protected]>
@@ -36,7 +36,7 @@ def visit_Attribute(
if (
_should_rewrite(state) and
isinstance(node.value, ast.Name) and
node.value.id == 'typing' and
state.as_imports.get(node.value.id) == 'typing' and
Copy link
Author

@stephenfin stephenfin Oct 24, 2024

Choose a reason for hiding this comment

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

Note that this breaks cases like we see in typing_pep604_test.py below, where we use typing without first importing it. I think that's okay since that code is already broken in real-life, but if we wanted to avoid making those changes we could trivially change this to:

Suggested change
state.as_imports.get(node.value.id) == 'typing' and
state.as_imports.get(node.value.id, node.value.id) == 'typing' and

@asottile
Copy link
Owner

aliases present all sorts of problems and so they are intentionally not supported.

@asottile asottile closed this Oct 24, 2024
@stephenfin
Copy link
Author

they are intentionally not supported.

I did my due diligence and couldn't find anything in the code or existing issues to suggest this. Additionally, I see support for aliased objects in multiple plugins. If there's a serious issue I've missed here than a little more context would have been a good learning experience for both me and anyone else that stumbles upon this issue. But fine 🤷 As always, been a pleasure interacting with you, Anthony 🙄

@stephenfin stephenfin deleted the pep585-import-aliases branch October 24, 2024 14:40
@asottile
Copy link
Owner

the only plug-in with support is the imports one because the result is unambiguous

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants