Implement meet for literal types; add tests for 'is_same_type' #6043
Conversation
This pull request implements meets for literal types and adds some corresponding tests. It also adds a test suite for the `is_same_type` method while we're at it. This test suite also lets you test each type's inherent `__eq__` and `__hash__`, since we do also use those throughout the code (especially for literal types).
|
Disclaimer: I don't think I still yet fully grok how meets work and how they're used in mypy. Hopefully the tests I added are good enough, but I could be wrong about that. |
There was a problem hiding this comment.
Thanks! This looks good, I just have two minor suggestions for tests.
Disclaimer: I don't think I still yet fully grok how meets work and how they're used in mypy.
There is nothing special (apart from the fact that there some bugs and white spots). If you are confused by the error message for cases where you would expect Callable[[<nothing>], X], then this special-casing is done on purpose to avoid confusing users with Incompatible argument, <nothing> expected.
| self.assert_meet(UnionType([lit1, lit2]), UnionType([lit2, lit3]), lit2) | ||
| self.assert_meet(UnionType([lit1, lit2]), UnionType([lit1, lit2]), UnionType([lit1, lit2])) | ||
| self.assert_meet(lit1, self.fx.anyt, lit1) | ||
| self.assert_meet(lit1, self.fx.o, lit1) |
There was a problem hiding this comment.
Maybe test these two in reverse order too? (i.e. if literal is the second one)
There was a problem hiding this comment.
The implementation of assert_meet already does this for us, thankfully: it's defined as just:
def assert_meet(self, s: Type, t: Type, meet: Type) -> None:
self.assert_simple_meet(s, t, meet)
self.assert_simple_meet(t, s, meet)
...where assert_simple_meet is responsible for actually doing the checking.
| reveal_type(unify(f3)) # E: Revealed type is 'Literal[1]' | ||
| reveal_type(unify(f4)) # E: Revealed type is 'Literal[1]' | ||
| reveal_type(unify(f5)) # E: Revealed type is 'Literal[1]' | ||
| [out] |
There was a problem hiding this comment.
TBH, having both these tests look a bit redundant (especially that you have very good unit tests). I would combine them into a single tests and check meeting some type one way, and some types another way.
There was a problem hiding this comment.
I consolidated both tests into one and pruned out some probably-overkill reveal_type(...) calls. I think testing the types in both ways looks tidier though, so kept that.
This pull request implements meets for literal types and adds some corresponding tests.
It also adds a test suite for the
is_same_typemethod while we're at it. This test suite also lets you test each type's inherent__eq__and__hash__, since we do also use those throughout the code (especially for literal types).