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

No error on new union syntax with union of fake types (TypeAlias and None) #11582

Open
KotlinIsland opened this issue Nov 20, 2021 · 5 comments
Labels
bug mypy got something wrong topic-pep-604 PEP 604 (union | operator)

Comments

@KotlinIsland
Copy link
Contributor

from typing import TypeAlias

AMONGUS: TypeAlias = "int"
IMPOSTER: TypeAlias = "str"

a: AMONGUS | IMPOSTER = 0  # SUS ALERT
b: AMONGUS | None = None  # SUS ALERT

Any types in a new union syntax that are fake(looking at you None) will be runtime errors.

@sobolevn
Copy link
Member

Interesting case 🤔

Forward references are allowed in https://www.python.org/dev/peps/pep-0613/#forward-references

But, I think that AMONGUS | IMPOSTER should be an error in mypy in this case.
We have to track what value was actually used for creating a type alias: real or forward ref.
And when using new-style unions we can check that | operator is supported for these types.

@sobolevn
Copy link
Member

This is quite hard to fix. Because AssignmentStmt does not have something like .annotation_expr.

Fields that are present at the moment:

mypy/mypy/nodes.py

Lines 1104 to 1121 in b47245a

lvalues: List[Lvalue]
# This is a TempNode if and only if no rvalue (x: t).
rvalue: Expression
# Declared type in a comment, may be None.
type: Optional["mypy.types.Type"]
# Original, not semantically analyzed type in annotation (used for reprocessing)
unanalyzed_type: Optional["mypy.types.Type"]
# This indicates usage of PEP 526 type annotation syntax in assignment.
new_syntax: bool
# Does this assignment define a type alias?
is_alias_def: bool
# Is this a final definition?
# Final attributes can't be re-assigned once set, and can't be overridden
# in a subclass. This flag is not set if an attempted declaration was found to
# be invalid during semantic analysis. It is still set to `True` if
# a final declaration overrides another final declaration (this is checked
# during type checking when MROs are known).
is_final_def: bool

So, my plan is:

  1. Add annotation_expr field to AssignmentStmt, assign it during fastparse.py
  2. Analyze it by slightly different rules during semanal.py
  3. Type check this new expression in checker.py stage
  4. Probably I would need to also modify things like ast merger / differ, fixup, cache, etc

I am not going to touch anything related to .type or .unanalyzed_type, so it should be fine.

@sobolevn
Copy link
Member

Moreover, we need to change how semanal binds types somehow for this case.
Because we need to make sure that semanal understands that AMONGUS: TypeAlias = "int" is str, not TypeAlias[int] when analyzed as an annotation.

@sobolevn
Copy link
Member

@KotlinIsland take a look at #11611

@KotlinIsland
Copy link
Contributor Author

@sobolevn Looks good! but does that handle None (None and strs are the only two fake types that I know of).
None | None currently is okay with mypy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-pep-604 PEP 604 (union | operator)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants