microsoft / pyright

Static Type Checker for Python
Other
13.12k stars 1.4k forks source link

Generic parameter of type Self not matched correctly with child classes #8713

Closed alexandermalyga closed 1 month ago

alexandermalyga commented 1 month ago

Describe the bug An error is raised when a child class calls a parent method that has a generic argument of Self. In this example FooEvent should be compatible with Event[Self] of Model because it's generic param is of type FooModel.

MyPy does not raise an error with this code (using --enable-incomplete-feature=NewGenericSyntax).

Code or Screenshots

from typing import Self

class Event[ModelT: Model]: ...

class Model:
    def fire_event(self, event: Event[Self]) -> None: ...

class FooEvent(Event["FooModel"]): ...

class FooModel(Model):
    def foo(self) -> None:
        self.fire_event(FooEvent())

Produces:

error: Argument of type "FooEvent" cannot be assigned to parameter "event" of type "Event[Self@FooModel]" in function "fire_event"
    "FooEvent" is incompatible with "Event[Self@FooModel]"
      Type parameter "ModelT@Event" is covariant, but "FooModel" is not a subtype of "Self@FooModel"
        Type "FooModel" is incompatible with type "Self@FooModel" (reportArgumentType)

VS Code extension or command-line

Running the command-line tool version 1.1.375

erictraut commented 1 month ago

Pyright's behavior is correct here, so this isn't a bug.

Mypy's behavior is non-conformant with the typing spec in this case. It should generate an error but does not due to known bugs.

Your Model.fire_event method uses the annotation Event[Self]. Self is effectively a type variable that represents the type of the self parameter. It has an upper bound of Model but can be any subclass of Model.

When you call Model.fire_event from FooModel.fire_event, you must pass a value of type Event[Self], but you are attempting to pass a value of type Event[FooModel]. Those are not the same. If this method were called from a subclass of FooModel, it would be different.

class FooModelSub(FooModel):
    def other(self) -> None:
        self.foo()

For more details about Self, refer to this section of the typing spec.

It sounds like your intent is for Model.fire_event to accept any Event[Model] or subtype thereof. If that's the case, then you should change the annotation for the event parameter from Event[Self] to Event["Model"].

alexandermalyga commented 1 month ago

Thanks for the quick response!

It sounds like your intent is for Model.fire_event to accept any Event[Model] or subtype thereof. If that's the case, then you should change the annotation for the event parameter from Event[Self] to Event["Model"].

My intent is to disallow models from firing events that are not related to themselves. FooModel should not be able to fire Event[BazModel] events, only Event[FooModel] ones, all while having a common Model.fire_event parent method for all model sub-classes. Changing the annotation to Event["Model"] would be too lenient.

Can this be achieved at all by just using the type system?

erictraut commented 1 month ago

You are currently parameterizing Event with a type variable that refers to the model. I think you can accomplish your goals if you swap that and parameterize Model with a type variable that refers to the event.

class Event:
    ...

class Model[EventT: Event]:
    def fire_event(self, event: EventT) -> None:
        ...

class FooEvent(Event):
    ...

class FooModel(Model[FooEvent]):
    def foo(self) -> None:
        self.fire_event(FooEvent())