-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix reassignment of generic TypeAliasType treated as type expression #20309
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
base: master
Are you sure you want to change the base?
Fix reassignment of generic TypeAliasType treated as type expression #20309
Conversation
This comment has been minimized.
This comment has been minimized.
hauntsaninja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding tests for the new behaviour? test-data/unit/check-python312.test is a good place to add them
@hauntsaninja Thanks for the review. I added the regression test to check-python312.test, but I am hitting a persistent test fixture error locally: AssertionError: Var(TypeVar). It seems the test environment defines TypeVar as a Var, but checkexpr.py expects it to be a TypeInfo (class) when analyzing the TypeAliasType assignment. I tried adding [builtins fixtures/list.pyi] and [builtins fixtures/dict.pyi] to the test case to load full definitions, but the assertion failure persists. Could you advise on which fixture is required for testing PEP 695 TypeAliasType assignments? Here is the test case I am adding: |
e7bda08 to
c1f3b1e
Compare
for more information, see https://pre-commit.ci
Quick update: I managed to resolve the fixture issue mentioned above. I found that explicitly adding Please let me know if any further changes are required. Thanks for your kind attention to this matter! |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
hauntsaninja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for adding the test cases and fighting the fixtures (they're the most annoying part about developing mypy).
Hmm, looking at your test case I think this prevents the use of B in type annotations entirely. That is, def foo(x: B) -> None: ... will now emit an error Variable "B" is not valid as a type. I'm guessing that's not what @jorenham expects
Maybe there is a solution that involves looking at whether the right hand side is a Python 3.12 type alias in typeanal.py, maybe somewhere around instantiate_type_alias
@hauntsaninja Thank you again for your patience and detailed guidance. I want to confirm the final implementation strategy before pushing: I will revert the logic in The fix will be implemented inside My understanding is that this solves the original assignment issue while preserving Does this core strategy align with your expectations? |
Fixes #20308
Summary
This PR fixes a false positive
[type-arg]error when reassigning a generic PEP 695TypeAliasTypeto a variable without type arguments.The Fix
Modified
is_type_refinsemanal.pyto returnFalsewhen the target is a genericTypeAliasTypeused without subscripts. This forces the analyzer to treat it as a variable assignment rather than a type definition.Verification
I verified this locally with a reproduction script.