Repository: python/mypy
Pull request: Bind self correctly when mapping class methods from supertype
Fixes https://github.com/python/mypy/issues/2511

Problem statement:
classmethod and selftype: incompatible with supertype
I am defining some classes capable of returning an (optional) instance of themselves based on a configuration (a dictionary).

In the base class I use the new SelfType capability to define the generic classmethod. I wanted to use a metaclass with abc.abstractclassmethod but mypy doesn't seem to support that.

In the subclass I now use the narrower type for cls and the return value but mypy gives me an error:

```sh
$ ./virtualenv/bin/python3 -m mypy --version
mypy 0.4.6
$ ./virtualenv/bin/python3 -m mypy repro.py 
repro.py: note: In class "B":
repro.py:15: error: Return type of "from_config" incompatible with supertype "A"
```

repro.py:
```Python
from typing import TypeVar, Type, Optional

AT = TypeVar('AT', bound='A')

class A:
    @classmethod 
    def from_config(cls: Type[AT], config: dict) -> Optional[AT]:
        return None

class B(A):
    @classmethod
    def from_config(cls: Type[B], config: dict) -> Optional[B]:
        return B()
```

I could only make it work by defining B like this which looks quite ugly and requires passing around the type variable:
```Python
from typing import cast

class B(A):
    @classmethod
    def from_config(cls: Type[AT], config: dict) -> Optional[AT]:
        return cast(AT, B())
```

Is that the intended result?

Hints from issue discussion:
This is a known issue with self types: #2353
Closed as duplicate.
I think this is not a duplicate of #2353. In that issue mypy allows unsafe override, while here mypy prohibits a safe override. Also this example is specific to `@classmethod`, with normal self type it works. Then it is probably easier to fix.
Still seeing this with mypy 0.670
#7401 has a slightly different example (no annotation for `cls` in the method override).
https://github.com/python/mypy/issues/7466 looks like yet another duplicate. Raising priority to high, since this started to come up more often and a fix might be not hard.

Commit message: Bind self correctly when mapping class methods from supertype

Patch to review:
diff --git a/mypy/checker.py b/mypy/checker.py
index 4dabdc6fdfcd..5eb36c81b38f 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -1530,7 +1530,13 @@ def bind_and_map_method(self, sym: SymbolTableNode, typ: FunctionLike,
         """
         if (isinstance(sym.node, (FuncDef, OverloadedFuncDef, Decorator))
                 and not is_static(sym.node)):
-            bound = bind_self(typ, self.scope.active_self_type())
+            if isinstance(sym.node, Decorator):
+                is_class_method = sym.node.func.is_class
+            elif isinstance(sym.node, OverloadedFuncDef):
+                is_class_method = sym.node.is_class
+            else:
+                is_class_method = False
+            bound = bind_self(typ, self.scope.active_self_type(), is_class_method)
         else:
             bound = typ
         return cast(FunctionLike, map_type_from_supertype(bound, sub_info, super_info))
diff --git a/test-data/unit/check-generics.test b/test-data/unit/check-generics.test
index c7866dbbedee..4a3829200f65 100644
--- a/test-data/unit/check-generics.test
+++ b/test-data/unit/check-generics.test
@@ -2062,3 +2062,52 @@ class A:
 x, y = A()
 reveal_type(x)  # N: Revealed type is 'Any'
 reveal_type(y)  # N: Revealed type is 'Any'
+
+[case testSubclassingGenericSelfClassMethod]
+from typing import TypeVar, Type
+
+AT = TypeVar('AT', bound='A')
+
+class A:
+    @classmethod
+    def from_config(cls: Type[AT]) -> AT:
+        ...
+
+class B(A):
+    @classmethod
+    def from_config(cls: Type[B]) -> B:
+        return B()
+[builtins fixtures/classmethod.pyi]
+
+[case testSubclassingGenericSelfClassMethodOptional]
+# flags: --strict-optional
+from typing import TypeVar, Type, Optional
+
+AT = TypeVar('AT', bound='A')
+
+class A:
+    @classmethod
+    def from_config(cls: Type[AT]) -> Optional[AT]:
+        return None
+
+class B(A):
+    @classmethod
+    def from_config(cls: Type[B]) -> Optional[B]:
+        return B()
+[builtins fixtures/classmethod.pyi]
+
+[case testSubclassingGenericSelfClassMethodNonAnnotated]
+from typing import TypeVar, Type
+
+AT = TypeVar('AT', bound='A')
+
+class A:
+    @classmethod
+    def from_config(cls: Type[AT]) -> AT:
+        ...
+
+class B(A):
+    @classmethod
+    def from_config(cls) -> B:
+        return B()
+[builtins fixtures/classmethod.pyi]


Code review comments:
diff_hunk: @@ -1530,7 +1530,13 @@ def bind_and_map_method(self, sym: SymbolTableNode, typ: FunctionLike,
         """
         if (isinstance(sym.node, (FuncDef, OverloadedFuncDef, Decorator))
                 and not is_static(sym.node)):
-            bound = bind_self(typ, self.scope.active_self_type())
+            if isinstance(sym.node, Decorator):
+                is_class_method = sym.node.func.is_class
+            elif isinstance(sym.node, OverloadedFuncDef):
+                is_class_method = sym.node.is_class
+            else:
+                is_class_method = False
path: mypy/checker.py
text: @user1:
Should we check `is_class` for regular `FuncDef`s? (It can be implicit for certain functions, right?)

@author:
Hm, you are right, it is better to be on the safe side.