Strip type objects when constructing TypeType #2832
Conversation
|
The title is somewhat misleading, it doesn't really strip all occurrences The need to check for |
elazarg
changed the title
| @@ -736,7 +736,7 @@ MypyFile:1( | |||
| f | |||
| Args( | |||
| Var(a)) | |||
| def (a: builtins.Any) -> builtins.Any | |||
| def (a: Any) -> Any | |||
There was a problem hiding this comment.
This change is new. I'm not comfortable with both the original and the new version.
|
I'm guessing those were from #3025, which has since been reverted. Can you do another rebase? Maybe things are simpler then. |
|
Only the last commit (not shown after the new rebase) was relevant. |
|
So are you now happy, or do you want to investigate why that test output had to be changed? I have no idea myself. |
|
This part of the test was added in #2603 two days ago, so I guess it's OK. |
|
Thanks! |
| @@ -384,6 +384,7 @@ class Instance(Type): | |||
|
|
|||
| def __init__(self, typ: Optional[mypy.nodes.TypeInfo], args: List[Type], | |||
| line: int = -1, column: int = -1, erased: bool = False) -> None: | |||
| assert(typ is None or typ.fullname() not in ["builtins.Any", "typing.Any"]) | |||
There was a problem hiding this comment.
This test is not right -- typ should never be None. What's the purpose of this?
There was a problem hiding this comment.
It is None in incremental mode
There was a problem hiding this comment.
Ah I figured that out myself, but my comment was lost somehow. What about builtins.Any and typing.Any? Why are we creating an Instance type that corresponds to Any, and why is there a builtins.Any in addition to typing.Any? Shouldn't we use AnyType?
There was a problem hiding this comment.
Yes we should, but these somehow got created in the tests. I have avoided this ( or fullname() == 'builtins' above), and added the assertions here to verify this is indeed the case. Perhaps we can remove the tests now.
As Guido said above - or at least as I understand his comment - it will be better not to define Any in builtins fixture at all.
Fix #2826,
I am not entirely sure this is the right fix, but removing a level of indirection seems reasonable. (I think the right thing, in the long run, is to remove TypeType completely from the system)
There are changes in testtypegen, which I do not completely understand but without which it kept adding
IntExpr(1)in the output - presumably from Any.