Incorrect inference with isinstance()

[pkch]

I think there are a few problems with isinstance inference.

1. This code type checks with mypy --strcit --python-version 3.6, but it should not:

from typing import *
def f(x: Union[int, str], typ: type) -> None:
    if isinstance(x, (typ, int)):
        print(x + 1)

The semantic analyzer concludes that within the body of if isinstance(), the type of x must be int. It's not, however. For example, in this case f('a', str), the if branch will be followed even though x is not an int (and this will cause runtime TypeError without any mypy warnings).

The reason this happens is that type annotation is represented as an Instance rather than as a FunctionLike inside checker.py:get_isinstance_type(). As a result, it's skipped entirely in the loop, and only int is added to types, so sem analyzer thinks x must absolutely be int.

One solution is to make type represented with an instance of CallableType (why isn't it btw?!). Another solution is to simply add a bit of logic to the loop to properly handle this case.

2. This code:

def f(x: Union[int, str, List, Tuple]) -> None:
    if isinstance(x, (str, (list, tuple))):
        print(x[1])

results in error: Argument 2 to "isinstance" has incompatible type "Tuple[str, Tuple[List[_T], Tuple[_T_co, ...]]]"; expected "Union[type, Tuple[type, ...]]".

This is because there's this weird thing in python that allows to put nested tuples as the second argument to isinstance, and mypy (along with most other people) doesn't know about it. I don't know if it's worth fixing, but it's super easy (just flatten the second argument if it's a tuple).

If my understanding is correct, I'll make a PR.

[JukkaL]

Yes, the way type objects are represented right now is kind of messy. Until we clean up the implementation, mypy should support all the possible representations of type objects in all contexts.

The second case would be a reasonable thing to fix if the fix is simple enough, though clearly it's not a high priority thing. Could you actually create a separate issue for the second case, since it seems like an independent issue?

[pkch]

Actually, I was wrong: the 1) issue has nothing to do with whether type is a CallableType or an Instance (and I understand now why it's not quite a good fit for CallableType). The problem is that from the perspective of type checker, the second argument to isinstance can be either a literal type (when the source code directly refers to a class name), or a variable that contains the type (when the source code uses a function argument or something else). In the second case, the type checker cannot really infer anything since the value inside the variable won't be known statically. So I don't think there's any need to fix the representation of type, just a small logic change to the function will fix the problem.

And yes, I can separate them out.

[JukkaL]

Ah yeah, I didn't read the first example correctly. Yes, if one the types is a type instance, it can match any type, but it could also not match any type. For example:

from typing import *
def f(x: Union[int, str], typ: type) -> None:
    if isinstance(x, (typ, int)):
        reveal_type(x)  # should be Union[int, str]
    else:
        reveal_type(x)  # should be str

[pkch]

I'm moving item 2) into a separate issue as you suggested.

[gvanrossum]

What's the issue# for the separate issue you created for (2)? Also does #2997 fix this or (2) or both?

[pkch]

#2997 would only fix (1). The separate issue I created for (2) is #2994.

[pkch]

To clarify: this issue is completely fixed in 2 merged PRs: #2997 and #2995.