Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invariance not enforced for class-scoped TypeVar used in base class expression #13713

Closed
erictraut opened this issue Sep 23, 2022 · 3 comments · Fixed by #13714
Closed

Invariance not enforced for class-scoped TypeVar used in base class expression #13713

erictraut opened this issue Sep 23, 2022 · 3 comments · Fixed by #13714
Assignees
Labels
bug mypy got something wrong topic-type-variables

Comments

@erictraut
Copy link

Mypy does not detect the case where a covariant or contravariant class (i.e. a class that is parameterized by a class-scoped covariant or contravariant TypeVar) derives from an invariant class. This creates a type correctness hole as shown in the following code sample.

from typing import TypeVar

T = TypeVar("T", covariant=True)

class MyList(list[T]): pass

def append_float(y: MyList[float]):
    y.append(1.0)

x: MyList[int] = MyList([1, 2, 3])
append_float(x)

Pyright likewise did not previously catch this case. I just added the check here.

@erictraut erictraut added the bug mypy got something wrong label Sep 23, 2022
@hauntsaninja hauntsaninja self-assigned this Sep 23, 2022
@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Sep 23, 2022

It's gotten late for me, so I'm now confused by variance. @erictraut I noticed you didn't implement the check for whether a covariant class derives from a contravariant class, and vice versa? I think we should do that as well, because it breaks transitivity of subtyping.

from typing import TypeVar, Generic
T = TypeVar('T')
T_co = TypeVar('T_co', covariant=True)
T_contra = TypeVar('T_contra', contravariant=True)

class A: ...
class B(A): ...

class Parent(Generic[T_co]): ...
class Child(Parent[T_contra], Generic[T_contra]): ...

def takes_parent_a(f: Parent[A]): ...
def takes_parent_b(f: Parent[B]): ...
def takes_child_a(f: Child[A]): ...
def takes_child_b(f: Child[B]): ...

def give_child_a(t: Child[A]):
    takes_parent_a(t)
    takes_parent_b(t)
    takes_child_a(t)
    takes_child_b(t)

def give_child_b(t: Child[B]):
    takes_parent_a(t)
    takes_parent_b(t)
    takes_child_a(t)
    takes_child_b(t)

I think both mypy and pyright agree that Child[A] is a Child[B] and Child[B] is a Parent[B], but that Child[A] is not a Parent[B].

It looks like typeshed breaks this with Container and SupportsGetItem (in the following example pyright gives something different because pyright narrows as per #2008 (comment) , but you could trick it into showing the same behaviour)...

from _typeshed import SupportsGetItem
from typing import Container

class A: ...
class B(A): ...

sgia: SupportsGetItem[A, None]
sgib: SupportsGetItem[B, None] = sgia
conb: Container[B] = sgib  # no error
conb = sgia  # error

cc @JelleZijlstra

@ilevkivskyi
Copy link
Member

Isn't this a duplicate of a (super-old) #736?

@erictraut
Copy link
Author

erictraut commented Sep 23, 2022

@hauntsaninja, yes, I agree that mismatched covariance/contravariance mismatch is also a problem. I think it's unlikely that anyone would attempt to use those combinations, but for completeness that check could be added. [Update: I modified pyright to include this check as well.] [Update 2: After claiming that this would be unlikely to happen, I see that typeshed violates this in one place — in the SupportsGetItem protocol class. Looks like you also discovered this when you were making your change to mypy.]

@ilevkivskyi, yes, this looks like a duplicate. Apologies, I didn't notice the existing bug. I'll close this one accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-type-variables
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants