microsoft / pyright

Static Type Checker for Python
Other
12.7k stars 1.35k forks source link

Pyright does not flag `isinstance()` when the type is a Union on Python<=3.9 #8304

Closed wch closed 1 week ago

wch commented 1 week ago

This is related to #8302.

The following code runs fine on Python>=3.10, but results in a runtime error on Python 3.9. This is the expected behavior for those versions, but it seems to me that Pyright should flag the issue when checking on 3.9?

from typing import Union

if isinstance(1, Union[int, str]):
    print("yes")

Output from Python 3.9:

❯ python test.py
Traceback (most recent call last):
  File "/Users/winston/test.py", line 6, in <module>
    if isinstance(1, Foo):
  File "/nix/store/lk7q3iiym8vs2synsw7qcdlvvjsp54gl-python3-3.9.19/lib/python3.9/typing.py", line 720, in __instancecheck__
    return self.__subclasscheck__(type(obj))
  File "/nix/store/lk7q3iiym8vs2synsw7qcdlvvjsp54gl-python3-3.9.19/lib/python3.9/typing.py", line 723, in __subclasscheck__
    raise TypeError("Subscripted generics cannot be used with"
TypeError: Subscripted generics cannot be used with class and instance checks

Pyright thinks it's OK:

❯ pyright test.py --pythonversion 3.9
0 errors, 0 warnings, 0 informations 

However, I could understand if this is considered expected behavior for Pyright.

The reason I bring this up is because in our code base, we do have cases like this where isinstance() is used with a union type, and I would have hoped that pyright would find these type errors, instead of us encountering them at runtime.

erictraut commented 1 week ago

Pyright is working as intended here, so I don't consider this a bug. I'm going to change this to an enhancement request and treat it as such.

IMO, it's really unfortunate that Python 3.10 added support for unions in isinstance. A "union" operator applies only to sets of types and therefore should be used only in type expressions. This is the only place in all of Python where a union is treated specially within a value expression. This non-standard behavior leads to a bunch of problems and requires special-casing in specifications and tooling. And this change was completely unnecessary because isinstance has always accepted tuples of classes.

I recommend avoiding the use of unions in isinstance if you want your code to be robust. Use tuples instead.

I'm going to decline this enhancement request because I think it's pretty niche and applies only to Python 3.9 and older. Python 3.9 is nearing the end of its service lifetime.

wch commented 1 week ago

Sounds good. Thanks for the explanation!