Fix member access on generic classes (#6418)

Fixes #3645
Fixes #1337
Fixes #5664

The fix is straightforward, I just add/propagate the bound type variable values by mapping to supertype.

I didn't find any corner cases with class methods, and essentially follow the same logic as when we generate the callable from `__init__` for generic classes in calls like `C()` or `C[int]()`.

For class attributes there are two things I fixed. First we used to prohibit ambiguous access:
```python
class C(Generic[T]):
    x: T
C.x  # Error!
C[int].x  # Error!
```
but the type variables were leaking after an error, now they are erased to `Any`. Second, I now make an exception and allow accessing attributes on `Type[C]`, this is very similar to how we allow instantiation of `Type[C]` even if it is abstract (because we expect concrete subclasses there), plus this allows accessing variables on `cls` (first argument in class methods), for example:
```python
class C(Generic[T]):
    x: T
    def get(cls) -> T:
        return cls.x  # OK
```

(I also added a bunch of more detailed comments in this part of code.)

mypy/checkmember.py

@@ -15,6 +15,7 @@
 from mypy.messages import MessageBuilder
 from mypy.maptype import map_instance_to_supertype
 from mypy.expandtype import expand_type_by_instance, expand_type, freshen_function_type_vars
+from mypy.erasetype import erase_typevars
 from mypy.infer import infer_type_arguments
 from mypy.typevars import fill_typevars
 from mypy.plugin import AttributeContext
@@ -621,11 +622,47 @@ def analyze_class_attribute_access(itype: Instance,
             symnode = node.node
             assert isinstance(symnode, Var)
             return mx.chk.handle_partial_var_type(t, mx.is_lvalue, symnode, mx.context)
-        if not is_method and (isinstance(t, TypeVarType) or get_type_vars(t)):
-            mx.msg.fail(message_registry.GENERIC_INSTANCE_VAR_CLASS_ACCESS, mx.context)
+
+        # Find the class where method/variable was defined.
+        if isinstance(node.node, Decorator):
+            super_info = node.node.var.info  # type: Optional[TypeInfo]
+        elif isinstance(node.node, (Var, FuncBase)):
+            super_info = node.node.info
+        else:
+            super_info = None
+
+        # Map the type to how it would look as a defining class. For example:
+        #     class C(Generic[T]): ...
+        #     class D(C[Tuple[T, S]]): ...
+        #     D[int, str].method()
+        # Here itype is D[int, str], isuper is C[Tuple[int, str]].
+        if not super_info:
+            isuper = None
+        else:
+            isuper = map_instance_to_supertype(itype, super_info)
+
+        if isinstance(node.node, Var):
+            assert isuper is not None
+            # Check if original variable type has type variables. For example:
+            #     class C(Generic[T]):
+            #         x: T
+            #     C.x  # Error, ambiguous access
+            #     C[int].x  # Also an error, since C[int] is same as C at runtime
+            if isinstance(t, TypeVarType) or get_type_vars(t):
+                # Exception: access on Type[...], including first argument of class methods is OK.
+                if not isinstance(mx.original_type, TypeType):
+                    mx.msg.fail(message_registry.GENERIC_INSTANCE_VAR_CLASS_ACCESS, mx.context)
+
+            # Erase non-mapped variables, but keep mapped ones, even if there is an error.
+            # In the above example this means that we infer following types:
+            #     C.x -> Any
+            #     C[int].x -> int
+            t = erase_typevars(expand_type_by_instance(t, isuper))
+
         is_classmethod = ((is_decorated and cast(Decorator, node.node).func.is_class)
                           or (isinstance(node.node, FuncBase) and node.node.is_class))
-        result = add_class_tvars(t, itype, is_classmethod, mx.builtin_type, mx.original_type)
+        result = add_class_tvars(t, itype, isuper, is_classmethod, mx.builtin_type,
+                                 mx.original_type)
         if not mx.is_lvalue:
             result = analyze_descriptor_access(mx.original_type, result, mx.builtin_type,
                                                mx.msg, mx.context, chk=mx.chk)
@@ -660,33 +697,54 @@ def analyze_class_attribute_access(itype: Instance,
         return function_type(cast(FuncBase, node.node), mx.builtin_type('builtins.function'))
 
 
-def add_class_tvars(t: Type, itype: Instance, is_classmethod: bool,
+def add_class_tvars(t: Type, itype: Instance, isuper: Optional[Instance], is_classmethod: bool,
                     builtin_type: Callable[[str], Instance],
                     original_type: Type) -> Type:
     """Instantiate type variables during analyze_class_attribute_access,
     e.g T and Q in the following:
 
-    def A(Generic(T)):
+    class A(Generic[T]):
         @classmethod
         def foo(cls: Type[Q]) -> Tuple[T, Q]: ...
 
-    class B(A): pass
+    class B(A[str]): pass
 
     B.foo()
 
     original_type is the value of the type B in the expression B.foo()
     """
     # TODO: verify consistency between Q and T
     info = itype.type  # type: TypeInfo
+    if is_classmethod:
+        assert isuper is not None
+        t = 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):
+    #     A       # The type of callable is def [T] () -> A[T], _not_ def () -> A[Any]
+    #     A[int]  # The type of callable is def () -> A[int]
+    # and
+    #     A.foo       # The type is generic def [T] () -> Tuple[T, A[T]]
+    #     A[int].foo  # The type is non-generic def () -> Tuple[int, A[int]]
+    #
+    # This behaviour is useful for defining alternative constructors for generic classes.
+    # To achieve such behaviour, we add the class type variables that are still free
+    # (i.e. appear in the return type of the class object on which the method was accessed).
+    free_ids = {t.id for t in itype.args if isinstance(t, TypeVarType)}
+
     if isinstance(t, CallableType):
-        # TODO: Should we propagate type variable values?
+        # NOTE: in practice either all or none of the variables are free, since
+        # visit_type_application() will detect any type argument count mismatch and apply
+        # a correct number of Anys.
         tvars = [TypeVarDef(n, n, i + 1, [], builtin_type('builtins.object'), tv.variance)
-                 for (i, n), tv in zip(enumerate(info.type_vars), info.defn.type_vars)]
+                 for (i, n), tv in zip(enumerate(info.type_vars), info.defn.type_vars)
+                 # use 'is' to avoid id clashes with unrelated variables
+                 if any(tv.id is id for id in free_ids)]
         if is_classmethod:
             t = bind_self(t, original_type, is_classmethod=True)
         return t.copy_modified(variables=tvars + t.variables)
     elif isinstance(t, Overloaded):
-        return Overloaded([cast(CallableType, add_class_tvars(item, itype, is_classmethod,
+        return Overloaded([cast(CallableType, add_class_tvars(item, itype, isuper, is_classmethod,
                                                               builtin_type, original_type))
                            for item in t.items()])
     return t

mypy/newsemanal/semanal.py

@@ -923,7 +923,7 @@ def analyze_class(self, defn: ClassDef) -> None:
         self.prepare_class_def(defn)
 
         defn.type_vars = tvar_defs
-        defn.info.type_vars = [tvar.fullname for tvar in tvar_defs]
+        defn.info.type_vars = [tvar.name for tvar in tvar_defs]
         if base_error:
             defn.info.fallback_to_any = True
 

test-data/unit/check-generics.test

@@ -1845,3 +1845,208 @@ def g(x: T) -> T: return x
 [out]
 main:3: error: Revealed type is 'def [b.T] (x: b.T`-1) -> b.T`-1'
 main:4: error: Revealed type is 'def [T] (x: T`-1) -> T`-1'
+
+[case testGenericClassMethodSimple]
+from typing import Generic, TypeVar
+T = TypeVar('T')
+
+class C(Generic[T]):
+    @classmethod
+    def get(cls) -> T: ...
+
+class D(C[str]): ...
+
+reveal_type(D.get())  # E: Revealed type is 'builtins.str*'
+reveal_type(D().get())  # E: Revealed type is 'builtins.str*'
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassMethodExpansion]
+from typing import Generic, TypeVar, Tuple
+T = TypeVar('T')
+
+class C(Generic[T]):
+    @classmethod
+    def get(cls) -> T: ...
+class D(C[Tuple[T, T]]): ...
+class E(D[str]): ...
+
+reveal_type(E.get())  # E: Revealed type is 'Tuple[builtins.str*, builtins.str*]'
+reveal_type(E().get())  # E: Revealed type is 'Tuple[builtins.str*, builtins.str*]'
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassMethodExpansionReplacingTypeVar]
+from typing import Generic, TypeVar
+T = TypeVar('T')
+S = TypeVar('S')
+
+class C(Generic[T]):
+    @classmethod
+    def get(cls) -> T: ...
+
+class D(C[S]): ...
+class E(D[int]): ...
+
+reveal_type(E.get())  # E: Revealed type is 'builtins.int*'
+reveal_type(E().get())  # E: Revealed type is 'builtins.int*'
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassMethodUnboundOnClass]
+from typing import Generic, TypeVar
+T = TypeVar('T')
+
+class C(Generic[T]):
+    @classmethod
+    def get(cls) -> T: ...
+    @classmethod
+    def make_one(cls, x: T) -> C[T]: ...
+
+reveal_type(C.get)  # E: Revealed type is 'def [T] () -> T`1'
+reveal_type(C[int].get)  # E: Revealed type is 'def () -> builtins.int*'
+reveal_type(C.make_one)  # E: Revealed type is 'def [T] (x: T`1) -> __main__.C[T`1]'
+reveal_type(C[int].make_one)  # E: Revealed type is 'def (x: builtins.int*) -> __main__.C[builtins.int*]'
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassMethodUnboundOnSubClass]
+from typing import Generic, TypeVar, Tuple
+T = TypeVar('T')
+S = TypeVar('S')
+
+class C(Generic[T]):
+    @classmethod
+    def get(cls) -> T: ...
+    @classmethod
+    def make_one(cls, x: T) -> C[T]: ...
+class D(C[Tuple[T, S]]): ...
+class E(D[S, str]): ...
+
+reveal_type(D.make_one)  # E: Revealed type is 'def [T, S] (x: Tuple[T`1, S`2]) -> __main__.C[Tuple[T`1, S`2]]'
+reveal_type(D[int, str].make_one)  # E: Revealed type is 'def (x: Tuple[builtins.int*, builtins.str*]) -> __main__.C[Tuple[builtins.int*, builtins.str*]]'
+reveal_type(E.make_one)  # E: Revealed type is 'def [S] (x: Tuple[S`1, builtins.str*]) -> __main__.C[Tuple[S`1, builtins.str*]]'
+reveal_type(E[int].make_one)  # E: Revealed type is 'def (x: Tuple[builtins.int*, builtins.str*]) -> __main__.C[Tuple[builtins.int*, builtins.str*]]'
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassMethodUnboundOnClassNonMatchingIdNonGeneric]
+from typing import Generic, TypeVar, Any, Tuple, Type
+
+T = TypeVar('T')
+S = TypeVar('S')
+Q = TypeVar('Q', bound=A[Any])
+
+class A(Generic[T]):
+    @classmethod
+    def foo(cls: Type[Q]) -> Tuple[T, Q]: ...
+
+class B(A[T], Generic[T, S]):
+    def meth(self) -> None:
+        reveal_type(A[T].foo)  # E: Revealed type is 'def () -> Tuple[T`1, __main__.A*[T`1]]'
+    @classmethod
+    def other(cls) -> None:
+        reveal_type(cls.foo)  # E: Revealed type is 'def [T, S] () -> Tuple[T`1, __main__.B*[T`1, S`2]]'
+reveal_type(B.foo)  # E: Revealed type is 'def [T, S] () -> Tuple[T`1, __main__.B*[T`1, S`2]]'
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassAttrUnboundOnClass]
+from typing import Generic, TypeVar
+T = TypeVar('T')
+
+class C(Generic[T]):
+    x: T
+    @classmethod
+    def get(cls) -> T:
+        return cls.x  # OK
+
+x = C.x  # E: Access to generic instance variables via class is ambiguous
+reveal_type(x)  # E: Revealed type is 'Any'
+xi = C[int].x  # E: Access to generic instance variables via class is ambiguous
+reveal_type(xi)  # E: Revealed type is 'builtins.int'
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassAttrUnboundOnSubClass]
+from typing import Generic, TypeVar, Tuple
+T = TypeVar('T')
+
+class C(Generic[T]):
+    x: T
+class D(C[int]): ...
+class E(C[int]):
+    x = 42
+
+x = D.x  # E: Access to generic instance variables via class is ambiguous
+reveal_type(x)  # E: Revealed type is 'builtins.int'
+E.x  # OK
+
+[case testGenericClassMethodOverloaded]
+from typing import Generic, TypeVar, overload, Tuple
+T = TypeVar('T')
+
+class C(Generic[T]):
+    @overload
+    @classmethod
+    def get(cls) -> T: ...
+    @overload
+    @classmethod
+    def get(cls, n: int) -> Tuple[T, ...]: ...
+    @classmethod
+    def get(cls, n: int = 0):
+        pass
+
+class D(C[str]): ...
+
+reveal_type(D.get())  # E: Revealed type is 'builtins.str'
+reveal_type(D.get(42))  # E: Revealed type is 'builtins.tuple[builtins.str]'
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassMethodAnnotation]
+from typing import Generic, TypeVar, Type
+T = TypeVar('T')
+
+class Maker(Generic[T]):
+    x: T
+    @classmethod
+    def get(cls) -> T: ...
+
+class B(Maker[B]): ...
+
+def f(o: Maker[T]) -> T:
+    if bool():
+        return o.x
+    return o.get()
+b = f(B())
+reveal_type(b)  # E: Revealed type is '__main__.B*'
+
+def g(t: Type[Maker[T]]) -> T:
+    if bool():
+        return t.x
+    return t.get()
+bb = g(B)
+reveal_type(bb)  # E: Revealed type is '__main__.B*'
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassMethodAnnotationDecorator]
+from typing import Generic, Callable, TypeVar, Iterator
+
+T = TypeVar('T')
+
+class Box(Generic[T]):
+    @classmethod
+    def wrap(cls, generator: Callable[[], T]) -> Box[T]: ...
+
+class IteratorBox(Box[Iterator[T]]): ...
+
+@IteratorBox.wrap  # E: Argument 1 to "wrap" of "Box" has incompatible type "Callable[[], int]"; expected "Callable[[], Iterator[<nothing>]]"
+def g() -> int:
+    ...
+[builtins fixtures/classmethod.pyi]
+
+[case testGenericClassMethodInGenericFunction]
+from typing import Generic, TypeVar
+T = TypeVar('T')
+S = TypeVar('S')
+
+class C(Generic[T]):
+    @classmethod
+    def get(cls) -> T: ...
+
+def func(x: S) -> S:
+    return C[S].get()
+[builtins fixtures/classmethod.pyi]

test-data/unit/check-inference.test

@@ -2602,8 +2602,8 @@ class C(A):
     x = ['12']
 
 reveal_type(A.x) # E: Revealed type is 'builtins.list[Any]'
-reveal_type(B.x) # E: Revealed type is 'builtins.list[builtins.int*]'
-reveal_type(C.x) # E: Revealed type is 'builtins.list[builtins.str*]'
+reveal_type(B.x) # E: Revealed type is 'builtins.list[builtins.int]'
+reveal_type(C.x) # E: Revealed type is 'builtins.list[builtins.str]'
 
 [builtins fixtures/list.pyi]