Repository navigation
assert isinstance check for a non-subtype confuses mypy #2776
Description
Consider a coding pattern where one chooses (for some reason) to type a function as accepting a broad argument type, but in the body to assert isinstance a narrower type, like this:
class A(object): pass
class B(A): pass
def foo(x):
# type: (A) -> None
reveal_type(x)
assert isinstance(x, B)
reveal_type(x)
Mypy recognizes the assert and narrows the type of x for the second reveal_type, as expected:
$ mypy --python-version 2.7 mypytest.py
mypytest.py:7: error: Revealed type is 'mypytest.A'
mypytest.py:9: error: Revealed type is 'mypytest.B'
But, suppose one makes a mistake, and either uses the wrong class, or forgets to inherit from A, like this:
class A(object): pass
class C(object): pass
def bar(x):
# type: (A) -> None
reveal_type(x)
assert isinstance(x, C)
reveal_type(x)
Now, mypy appears to just ignore the assert and the second reveal_type:
$ mypy --python-version 2.7 mypytest2.py
mypytest2.py:7: error: Revealed type is 'mypytest2.A'
I recognize that multiple inheritance in Python means that mypy would need intersection types in order to type x precisely in the second example, but I am very surprised that it just completely ignores the reveal_type.
Activity
JukkaL commentedon Jan 31, 2017
@axch Mypy currently believes that the assert will always raise, so the following statements will never be reached. I agree that this is pretty surprising behavior. To generally fix this we'd need intersection types, as you correctly point out. However, in some cases inferring an intersection type would not be the right thing. Consider this code:
Now the revealed type should probably be
bytes, since you can't have multiple inheritance fromstrandbytes.I'm not sure what we should do in the short term -- intersection types are probably a lot of work to implement, so I doubt that's feasible right now.
Here are some ideas:
xthe typeCafter theisinstancecheck. This wouldn't always do the right thing, but it would probably be less surprising than what we have right now.TypeInfothat fakes an intersection type through multiple inheritance. This would be easier than full intersection type support, but still non-trivial. We'd still need to figure out what to do in thestr/bytescase.xthe typeAnyafter the assert so that we could do some type checking (or perhapsC).xthe typeAny. This would arguably be less bad than what we currently have, though still not great.axch commentedon Jan 31, 2017
In my case, warning about the assert would have been ideal, since that's where my mistake actually was. Or, at least, warning about an "unreachable"
reveal_typecall.It occurs to me that what the "right thing" to do in general is depends on a question about mypy's philosophy, which I have asked in the separate ticket #2780.
axch commentedon Feb 1, 2017
Another oddity: The HTML report generated by
--html-reportclassifies all lines after theassertas "line-empty", which causes them to appear visually as covered by type checking even though they are not. I think it would be much more helpful if the coverage report claimed that those lines were not covered.JukkaL commentedon Feb 1, 2017
@axch Can you file a separate issue for the HTML report problem?
axch commentedon Feb 1, 2017
Done.
JukkaL commentedon Feb 1, 2017
Thanks!
JukkaL commentedon Jan 28, 2020
Looks like this was fixed by #8305.