microsoft / pyright

Static Type Checker for Python
Other
13.41k stars 1.47k forks source link

Runtime TypeError caused by mixing `typing` and `typing_extensions` not caught #4380

Closed not-my-profile closed 1 year ago

not-my-profile commented 1 year ago

The last line of the following code results in the runtime TypeError:

TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

import typing
import typing_extensions

class Foo(typing.TypedDict): ...

class Bar(typing_extensions.TypedDict): ...

class Baz(Foo, Bar): ...

I just ran into this runtime error because I always default to using types from typing but sometimes use types from typing_extensions to support older versions of Python. In particular typing.TypedDict is incompatible with typing.Generic in Python < 3.11, see https://github.com/python/cpython/pull/27663#issuecomment-1187917727, so I sometimes used typing_extensions.TypedDict instead of typing.TypedDict.

It would be nice if pyright could detect this type error, which can be quite sneaky.

I think there could even be an additional optional check that warns about this issue whenever a project uses both typing.TypedDict and typing_extensions.TypedDict in the same code base (because otherwise the public API might have TypedDict classes that are incompatible with each other).

erictraut commented 1 year ago

There's nothing in the type system that would indicate that this runtime error would occur, so there's not much pyright could do about detecting this without hard-coding some knowledge about internal workings of the associated modules and classes. We generally don't do that.

The goal of pyright isn't to catch all bugs in your code. It's a static type checker, and it's meant to help you create more maintainable and robust code.

not-my-profile commented 1 year ago

But pyright already has special detection for this:

import typing

class Xyz:
    pass

class Foo(typing.TypedDict, Xyz): ... # error: All base classes for TypedDict classes must also be TypedDict classes

My point is that the current detection isn't working correctly.

not-my-profile commented 1 year ago

I agree that the second check I had in mind is out of scope for a static type checker and better implemented by a linter, so I just opened a respective issue at the ruff linter: charliermarsh/ruff/issues/1422

If you ban typing.TypedDict via a lint, then the reported TypeError cannot occur anymore anyway, so I think you made the right call by closing this issue :)