Skip to content

Commit

Permalink
Make logic around bind_self() consistent in different code paths (#8021)
Browse files Browse the repository at this point in the history
Fixes #8020

There is a bunch of code/logic duplication around `bind_self()`, mostly because of #7724. This PR makes all three main code paths consistently follow the same structure:
1. `freshen_function_type_vars()`
2. `bind_self()`
3. `expand_type_by_instance(..., map_instance_to_supertype())` (a.k.a `map_type_from_supertype()`)

I briefly scrolled through other code paths, and it looks like this was last major/obvious inconsistency (although code around `__getattr__`/`__setattr__`/`__get__`/`__set__` looks a bit suspicious).
  • Loading branch information
ilevkivskyi authored Nov 27, 2019
1 parent 596cde6 commit 1122fc6
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 10 deletions.
7 changes: 4 additions & 3 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -829,9 +829,7 @@ class B(A[str]): pass
variables of the class callable on which the method was accessed.
"""
# TODO: verify consistency between Q and T
if is_classmethod:
assert isuper is not None
t = get_proper_type(expand_type_by_instance(t, isuper))

# We add class type variables if the class method is accessed on class object
# without applied type arguments, this matches the behavior of __init__().
# For example (continuing the example in docstring):
Expand All @@ -847,7 +845,10 @@ class B(A[str]): pass
if isinstance(t, CallableType):
tvars = original_vars if original_vars is not None else []
if is_classmethod:
t = freshen_function_type_vars(t)
t = bind_self(t, original_type, is_classmethod=True)
assert isuper is not None
t = cast(CallableType, expand_type_by_instance(t, isuper))
return t.copy_modified(variables=tvars + t.variables)
elif isinstance(t, Overloaded):
return Overloaded([cast(CallableType, add_class_tvars(item, itype, isuper, is_classmethod,
Expand Down
12 changes: 6 additions & 6 deletions test-data/unit/check-generics.test
Original file line number Diff line number Diff line change
Expand Up @@ -2062,7 +2062,7 @@ class Base(Generic[T]):
return (cls(item),)
return cls(item)

reveal_type(Base.make_some) # N: Revealed type is 'Overload(def [T] (item: T`1) -> __main__.Base*[T`1], def [T] (item: T`1, n: builtins.int) -> builtins.tuple[__main__.Base*[T`1]])'
reveal_type(Base.make_some) # N: Revealed type is 'Overload(def [T] (item: T`1) -> __main__.Base[T`1], def [T] (item: T`1, n: builtins.int) -> builtins.tuple[__main__.Base[T`1]])'
reveal_type(Base.make_some(1)) # N: Revealed type is '__main__.Base[builtins.int*]'
reveal_type(Base.make_some(1, 1)) # N: Revealed type is 'builtins.tuple[__main__.Base[builtins.int*]]'

Expand Down Expand Up @@ -2100,11 +2100,11 @@ class A(Generic[T]):

class B(A[T], Generic[T, S]):
def meth(self) -> None:
reveal_type(A[T].foo) # N: Revealed type is 'def () -> Tuple[T`1, __main__.A*[T`1]]'
reveal_type(A[T].foo) # N: Revealed type is 'def () -> Tuple[T`1, __main__.A[T`1]]'
@classmethod
def other(cls) -> None:
reveal_type(cls.foo) # N: Revealed type is 'def () -> Tuple[T`1, __main__.B*[T`1, S`2]]'
reveal_type(B.foo) # N: Revealed type is 'def [T, S] () -> Tuple[T`1, __main__.B*[T`1, S`2]]'
reveal_type(cls.foo) # N: Revealed type is 'def () -> Tuple[T`1, __main__.B[T`1, S`2]]'
reveal_type(B.foo) # N: Revealed type is 'def [T, S] () -> Tuple[T`1, __main__.B[T`1, S`2]]'
[builtins fixtures/classmethod.pyi]

[case testGenericClassAlternativeConstructorPrecise]
Expand Down Expand Up @@ -2171,8 +2171,8 @@ class C(Generic[T]):

class D(C[str]): ...

reveal_type(D.get()) # N: Revealed type is 'builtins.str'
reveal_type(D.get(42)) # N: Revealed type is 'builtins.tuple[builtins.str]'
reveal_type(D.get()) # N: Revealed type is 'builtins.str*'
reveal_type(D.get(42)) # N: Revealed type is 'builtins.tuple[builtins.str*]'
[builtins fixtures/classmethod.pyi]

[case testGenericClassMethodAnnotation]
Expand Down
78 changes: 77 additions & 1 deletion test-data/unit/check-selftype.test
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ class A(Generic[T]):

t: Type[Union[A[int], A[str]]]
x = t.meth()
reveal_type(x) # N: Revealed type is 'Union[__main__.A*[builtins.int], __main__.A*[builtins.str]]'
reveal_type(x) # N: Revealed type is 'Union[__main__.A[builtins.int], __main__.A[builtins.str]]'
[builtins fixtures/classmethod.pyi]

[case testSelfTypeClassMethodOnUnionList]
Expand Down Expand Up @@ -1055,3 +1055,79 @@ class Concrete(Blah):
def something(self) -> None: ...

Concrete() # OK

[case testSelfTypeGenericClassNoClashInstanceMethod]
from typing import TypeVar, Generic

M = TypeVar("M")
T = TypeVar("T")
S = TypeVar("S")

class Descriptor(Generic[M]): ...

class BaseWrapper(Generic[M]):
def create_wrapper(self: T, metric_descriptor: Descriptor[M]) -> T: ...
class SubWrapper(BaseWrapper[M]): ...

def build_wrapper(descriptor: Descriptor[M]) -> BaseWrapper[M]:
wrapper: BaseWrapper[M]
return wrapper.create_wrapper(descriptor)

def build_sub_wrapper(descriptor: Descriptor[S]) -> SubWrapper[S]:
wrapper: SubWrapper[S]
x = wrapper.create_wrapper(descriptor)
reveal_type(x) # N: Revealed type is '__main__.SubWrapper[S`-1]'
return x

[case testSelfTypeGenericClassNoClashClassMethod]
from typing import TypeVar, Generic, Type

M = TypeVar("M")
T = TypeVar("T")
S = TypeVar("S")

class Descriptor(Generic[M]): ...

class BaseWrapper(Generic[M]):
@classmethod
def create_wrapper(cls: Type[T], metric_descriptor: Descriptor[M]) -> T: ...
class SubWrapper(BaseWrapper[M]): ...

def build_wrapper(descriptor: Descriptor[M]) -> BaseWrapper[M]:
wrapper_cls: Type[BaseWrapper[M]]
return wrapper_cls.create_wrapper(descriptor)

def build_sub_wrapper(descriptor: Descriptor[S]) -> SubWrapper[S]:
wrapper_cls: Type[SubWrapper[S]]
x = wrapper_cls.create_wrapper(descriptor)
reveal_type(x) # N: Revealed type is '__main__.SubWrapper[S`-1]'
return x
[builtins fixtures/classmethod.pyi]

[case testSelfTypeGenericClassNoClashClassMethodClassObject]
from typing import TypeVar, Generic, Type

M = TypeVar("M")
T = TypeVar("T")

class Descriptor(Generic[M]): ...

class BaseWrapper(Generic[M]):
@classmethod
def create_wrapper(cls: Type[T], metric_descriptor: Descriptor[M]) -> T: ...
class SubWrapper(BaseWrapper[M]): ...

def build_wrapper(descriptor: Descriptor[M]) -> BaseWrapper[M]:
return BaseWrapper.create_wrapper(descriptor)

def build_sub_wrapper(descriptor: Descriptor[M]) -> SubWrapper[M]:
x = SubWrapper.create_wrapper(descriptor)
reveal_type(x) # N: Revealed type is '__main__.SubWrapper[M`-1]'
return x

def build_wrapper_non_gen(descriptor: Descriptor[int]) -> BaseWrapper[str]:
return BaseWrapper.create_wrapper(descriptor) # E: Argument 1 to "create_wrapper" of "BaseWrapper" has incompatible type "Descriptor[int]"; expected "Descriptor[str]"

def build_sub_wrapper_non_gen(descriptor: Descriptor[int]) -> SubWrapper[str]:
return SubWrapper.create_wrapper(descriptor) # E: Argument 1 to "create_wrapper" of "BaseWrapper" has incompatible type "Descriptor[int]"; expected "Descriptor[str]"
[builtins fixtures/classmethod.pyi]

0 comments on commit 1122fc6

Please sign in to comment.