Skip to content

Now TypeVar with values correctly handles is_subtype check #11378

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Now TypeVar with values correctly handles is_subtype check #11378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5af8b75
Now `TypeVar` with values correctly handles `is_subtype` check
sobolevn Oct 24, 2021
a7d3a34
Fixes subtypes condition
sobolevn Oct 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions mypy/subtypes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,9 @@ def visit_instance(self, left: Instance) -> bool:
if call:
return self._is_subtype(call, right)
return False
else:
return False
if isinstance(right, TypeVarType) and right.values:
return any(self._is_subtype(left, r_val) for r_val in right.values)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the correct fix. Now this invalid code generates no error:

from typing import AnyStr, Generic

class Foo(Generic[AnyStr]):
    def method1(self, s: AnyStr, t: AnyStr) -> None:
        print(s)

class Bar(Foo[AnyStr]):
    def method1(self, s: AnyStr, t: AnyStr) -> None:
        print('Child before')
        super().method1('x', b'y')  # Should be an error
        print('Child after')

We type check the method by substituting all instances of AnyStr in the method first with str, and later with bytes. Currently we don't perform the substitution in SuperExpr, which seems to be the root cause of the problem.

SuperExpr should perhaps be modified to support type variable substitution somehow. For example, we could add a list of (type variable, substitution) tuples as an attribute of SuperExpr that are initially empty, but as substitutions are performed, we'd add an item to the list. When inferring the type of SuperExpr, we'd perform these substitutions. This way the type of super() could effectively be treated as Foo[str] or Foo[bytes] instead of Foo[AnyStr], which should make the original error go away.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see 😞

I will refactor this one to expand SuperExpr() 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more idea: we can represent method1 as an @overload of two functions:

  • def method1(self, s: str, t: str) -> None:
  • def method1(self, s: bytes, t: bytes) -> None:

So, this will help us to represent correct argument combinations.
What do you think?

return False

def visit_type_var(self, left: TypeVarType) -> bool:
right = self.right
Expand Down
13 changes: 13 additions & 0 deletions test-data/unit/check-typevar-values.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,3 +631,16 @@ def g(s: S) -> Callable[[S], None]: ...
def f(x: S) -> None:
h = g(x)
h(x)

[case testTypeVarWithValuesAndSuper]
from typing import Generic, TypeVar

AnyStr = TypeVar('AnyStr', bytes, str)

class Foo(Generic[AnyStr]):
def method1(self, s: AnyStr) -> None:
pass

class Bar(Foo[AnyStr]):
def method1(self, s: AnyStr) -> None:
super().method1(s)