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

Fix issues with type aliases and new style unions #14181

Merged
merged 16 commits into from
Nov 25, 2022
Merged

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Nov 24, 2022

Fix aliases like this and other aliases involving new-style unions:

A = type[int] | str

Fixes #12392. Fixes #14158.

@github-actions

This comment has been minimized.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Looks good, but there is one important comment.

@@ -1984,6 +1994,8 @@ class OpExpr(Expression):
right_always: bool
# Per static analysis only: Is the right side unreachable?
right_unreachable: bool
# Used for expressions that represent a type "X | Y" in some contexts
analyzed: TypeAliasExpr | None
Copy link
Member

Choose a reason for hiding this comment

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

This should now be handled in various visitors. Three cases come to my mind: First, treetransform.py this is needed so that this error will not re-appear in a test case like this

T = TypeVar("T", int, str)
def foo(x: T) -> T:
    A = type[int] | str
    return x

Second, aststrip.py, be sure that when you switch imported names from types to variables, you do get an error about missing __or__ on update. Third, semanal_typeargs.py (uses MixedTraverserVisitor), since we should not carry malformed instances around (with number of type args), they may cause crashes, add a test just in case with a malformed instance in | alias.

And in general adding in to the basic TraverserVisitor is a good idea. Maybe just grep for def visit_index_expr( and see where we use analyzed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Very good points! I'll fix these.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pydantic (https://github.com/samuelcolvin/pydantic)
+ pydantic/networks.py:16: error: Unused "type: ignore" comment

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