Fix class method type variables #7862
Conversation
…type-vars Conflicts: mypy/checkmember.py
| def foo(x: T, y: int) -> T: | ||
| C(y) # OK | ||
| C[T](y) # E: Argument 1 to "C" has incompatible type "int"; expected "T" | ||
| C[T].f(y) # E: Argument 1 to "f" of "C" has incompatible type "int"; expected "T" |
There was a problem hiding this comment.
Maybe also test C[T].f(x).
| @@ -484,9 +484,12 @@ class A(Generic[T]): | |||
| @classmethod | |||
| def foo(cls) -> None: | |||
| reveal_type(cls) # N: Revealed type is 'Type[__main__.A[T`1]]' | |||
| reveal_type(cls(1)) # N: Revealed type is '__main__.A[builtins.int*]' | |||
| reveal_type(cls('wooooo')) # N: Revealed type is '__main__.A[builtins.str*]' | |||
| reveal_type(cls(cls.x)) # N: Revealed type is '__main__.A[T`1]' | |||
There was a problem hiding this comment.
I think that this should actually be an error, since we refer to a generic attribute using the type object.
There was a problem hiding this comment.
There is exclusion to specifically support a common pattern like this:
class Base(Generic[T]):
impl_class: Type[T] # to be defined on subclasses
@classmethod
def make(cls) -> T:
return cls.impl_class()IIUC you want to prohibit this:
class C(Generic[T]):
def __init__(self, x: T) -> None:
self.x = x
@classmethod
def meth(cls) -> None:
cls.x # This should be banned as ambiguousIf yes, then I can try this (although may require a bit of special-casing for dataclasses where the variable appears in class body).
There was a problem hiding this comment.
Yeah, the first example looks reasonable, while the second one is not. Not sure what's the best way to deal with it.
There was a problem hiding this comment.
OK, I added the check for this an no tests failed, if people will complain about this it will be easy to revert, the change is very simple.
| @@ -240,7 +240,8 @@ def analyze_type_callable_member_access(name: str, | |||
| # This check makes sure that when we encounter an operator, we skip looking up | |||
| # the corresponding method in the current instance to avoid this edge case. | |||
| # See https://github.com/python/mypy/pull/1787 for more info. | |||
| result = analyze_class_attribute_access(ret_type, name, mx) | |||
| result = analyze_class_attribute_access(ret_type, name, mx, | |||
| original_vars=typ.items()[0].variables) | |||
There was a problem hiding this comment.
I wonder if picking the first item may cause problems with overloaded methods. If so, it would be good to add a comment and maybe create a follow-up issue (unless the fix is trivial).
There was a problem hiding this comment.
I think I use thus because I noticed overloaded constructors have "uniform" type variables, but maybe it is not guaranteed, I will add a test case and a TODO here.
There was a problem hiding this comment.
Funny enough, when adding a test for this (that seem to work well as I said) I discovered a bug because dozen lines above we use ret_type = typ.items()[0].ret_type and now this is clearly wrong.
| override_info: Optional[TypeInfo] = None) -> Optional[Type]: | ||
| override_info: Optional[TypeInfo] = None, | ||
| original_vars: Optional[List[TypeVarDef]] = None | ||
| ) -> Optional[Type]: |
There was a problem hiding this comment.
Document the new argument.
mypy/checkmember.py
Outdated
| @@ -799,7 +803,6 @@ class B(A[str]): pass | |||
| component in case if a union (this is used to bind the self-types). | |||
There was a problem hiding this comment.
Also discuss original_vars.
| if bool(): | ||
| return cls(0), cls(0) # E: Argument 1 to "Base" has incompatible type "int"; expected "T" | ||
| return cls(item), cls(item) | ||
| [builtins fixtures/classmethod.pyi] |
There was a problem hiding this comment.
What about testing an overloaded generic class method?
There was a problem hiding this comment.
This may interfere with #7926, if yes, then I will add a test in the other PR fixing that issue.
Fixes #7846
The fix is mostly straightforward, instead of ad-hoc guessing of type variables from the return type, we pass the them all the way through intermediate helper functions.
The
ctypesplugin change is because it used the bare representation of types (the one used byreveal_type()) for user errors. So after my change arrangement of stars changed, instead of adjusting it, I switched to the proper type formatting.