microsoft / pyright

Static Type Checker for Python
Other
13.25k stars 1.43k forks source link

Abnormal behavior when overriding `__getitem__` of a metaclass. #5489

Closed cainmagi closed 1 year ago

cainmagi commented 1 year ago

Describe the bug

Previously, this issue does not exist, and the overriding of metaclass __getitem__ works as my expectation perfectly. However, this behavior has changed since one week ago. Today I have updated my Pylance to 2023.7.10 (with pyright 74d8f3c4). But this issue still exists.

The usage discussed in this issue is for defining customized annotation types like Sequence[...].

By the way, Mypy has the same issue, which has been reported recently in https://github.com/python/mypy/issues/1771

Codes, screenshots, and how to Reproduce

from typing import Union, Sequence, TypeVar
from typing_extensions import reveal_type

T = TypeVar("T")

class TestMeta(type):
    def __getitem__(self, key: T) -> T:
        ...

class Test(metaclass=TestMeta):
    ...

val = Test[int]
reveal_type(val)  # val is annotated as `type[int]` as expectation.

val2 = Test[Sequence[int]]
reveal_type(val2)  # val2 is annotated as `type[Sequence[int]]` as expectation.

# A strange error is thrown saying, "Expected no type arguments for class Test"
val3 = Sequence[Test[int]]
# `val3` is annotated as `type[Sequence[Test]]`, however, we expect to get
# `type[Sequence[int]]`.
reveal_type(val3)

val4 = Union[Test[int], str]
# Annotated as `type[Test] | type[str]`, expect `type[int] | type[str]`.
reveal_type(val4)

val5 = Test[int] | str  # Need Python 3.10
reveal_type(val5)  # Annotated as `type[int] | type[str]` as expectation.
Click this to see the screenshot :framed_picture: ![image](https://github.com/microsoft/pyright/assets/32186723/9f47c1e5-6569-4d92-8765-0446fbf8544c)

It is not understandable why Test[int] can work solely but cannot be used as the argument of other annotations.

What is more confusing is that Union[Test[int], str] and Test[int] | str has two different behaviors in the above example. I know that writing such codes look a little bit like "hacking" the annotation features. However, no matter whether this coding style is approved or not, at least these two implementations should have the same behavior.

A partial solution that is not working

At least, I found a solution that can disable the error raised in the above example. However, the returned annotation type is still not correct.

from typing import Sequence, TypeVar
from typing_extensions import reveal_type

T = TypeVar("T")

class TestMeta(type):
    def __getitem__(self, key: T) -> T:
        ...

class Test(metaclass=TestMeta):
    @classmethod
    def __class_getitem__(cls, key: T) -> T:
        ...

val3 = Sequence[Test[int]]  # No error is raised here.
# `val3` is still annotated as `type[Sequence[Test]]`. Expecting `type[Sequence[int]]`.
reveal_type(val3)
Click this to see the screenshot :framed_picture: ![image](https://github.com/microsoft/pyright/assets/32186723/3755dd1e-3445-46c1-bdc6-a939394651bc)

Providing Test.__class_getitem__ only suppresses the type-checking error of using Test[int] as the argument of Sequence[...]. However, the returned value is still type[Sequence[Test]] which cannot be compatible with type[Sequence[int]].

Expected behavior

from typing import Union, Sequence, TypeVar
from typing_extensions import reveal_type

T = TypeVar("T")

class TestMeta(type):
    def __getitem__(self, key: T) -> T:
        ...

class Test(metaclass=TestMeta):
    ...

val = Test[int]
reveal_type(val)  # val is annotated as `type[int]` as expectation.

val2 = Test[Sequence[int]]
reveal_type(val2)  # val2 is annotated as `type[Sequence[int]]` as expectation.

val3 = Sequence[Test[int]]  # Should be no errors here.
reveal_type(val3)  # Expect `type[Sequence[int]]`.

val4 = Union[Test[int], str]  # Should be no errors here.
reveal_type(val4)  # Expect `type[int] | type[str]`.

val5 = Test[int] | str  # Need Python 3.10
reveal_type(val5)  # Annotated as `type[int] | type[str]` as expectation.
erictraut commented 1 year ago

I think your use of __getitem__ on a metaclass is questionable. It conflicts with the normal semantics of generics in Python static types.

Of course, with Python you can do all sorts of hacks at runtime that override standard behaviors, but some hacks will interfere with (and prevent proper operation of) static type checking. This may fall into that category.

You appear to be using the __getitem__ like a generic type argument in expressions such as Union[Test[int], str]. Normally the statement val4 = Union[Test[int], str] would define a type alias called val4. Is that your intent here? If so, it should probably be considered an invalid type expression because Test[int] is not a valid type annotation given the way you're overriding the metaclass __getitem__.

Unsurprisingly, mypy emits the same errors in this case. I think both pyright and mypy are arguably correct in doing so here.

cainmagi commented 1 year ago

I think your use of __getitem__ on a metaclass is questionable. It conflicts with the normal semantics of generics in Python static types.

Of course, with Python you can do all sorts of hacks at runtime that override standard behaviors, but some hacks will interfere with (and prevent proper operation of) static type checking. This may fall into that category.

You appear to be using the __getitem__ like a generic type argument in expressions such as Union[Test[int], str]. Normally the statement val4 = Union[Test[int], str] would define a type alias called val4. Is that your intent here? If so, it should probably be considered an invalid type expression because Test[int] is not a valid type annotation given the way you're overriding the metaclass __getitem__.

Unsurprisingly, mypy emits the same errors in this case. I think both pyright and mypy are arguably correct in doing so here.

Here I am only trying to make a simple example. I am just trying to find a way to define a customized annotation. Using Generic can only make an annotation referring to the newly defined class. I want to find more flexible usages.

The above example can be meaningful, for example, Test[T] can be viewed as a "proxy" of any type T. I know that there are some packages like wrapt doing such things. For example:

from typing import Generic, TypeVar, Any

T = TypeVar("T")

class Proxy(Generic[T]):
    def __init__(self, obj: T) -> None:
        object.__setattr__(self, "object", obj)

    def __repr__(self) -> str:
        return repr(object.__getattribute__(self, "object"))

    def __getitem__(self, __key) -> str:
        return object.__getattribute__(self, "object").__getitem__(__key)

    def __setitem__(self, __key, __val) -> str:
        return object.__getattribute__(self, "object").__setitem__(__key, __val)

    def __getattribute__(self, __name: str) -> Any:
        return getattr(object.__getattribute__(self, "object"), __name)

    def __setattr__(self, __name: str, __value: Any) -> None:
        return setattr(object.__getattribute__(self, "object"), __name, __value)

class Test:
    def method(self) -> None:
        ...

px = Proxy(Test())

def func(x: Test) -> None:
    ...

func(px)  # func is incompatible with px

In the above example, I make px as a proxy of Test. Since px supports all functionalities of Test(), I expect that px can be used as the argument of func(). However, the type checker will mark this as an error. If I cannot make Proxy[Test] compatible with Test, I may have to replace the argument x of the func with a protocol type.

Do you mean that such "customized annotation" will not be approved? If so, I may need to consider how to solve such issues without hacking the annotations in the future...

erictraut commented 1 year ago

I am just trying to find a way to define a customized annotation.

Then my suspicion was correct. You are abusing the static type system here. This won't work with static type checkers.