Repository navigation
Convert type[Self] to callable from class constructor - #5592
crypt0lith wants to merge 2 commits into
Conversation
|
This pull request has been imported. If you are a Meta employee, you can view this in D130651720. (Because this pull request was imported automatically, there will not be any future comments.) |
stroxler
left a comment
There was a problem hiding this comment.
Thanks for the PR! I think working on a class-constructor-to-callable hook makes sense.
But it looks like the current behavior has some gaps. Consider this code, which uses a type variable rather than typing.Self:
from typing import Callable, Self, TypeVar, reveal_type
def relay[**P, R](f: Callable[P, R]) -> Callable[P, R]:
return f
class C:
def __new__[S: C](cls: type[S], x: int) -> S:
raise NotImplementedError
@classmethod
def make(cls) -> Self:
reveal_type(relay(cls))
return relay(cls)(1)
_S = TypeVar('_S', bound='Legacy')
class Legacy:
def __new__(cls: type[_S], x: int) -> _S:
raise NotImplementedError
@classmethod
def make(cls) -> Self:
reveal_type(relay(cls))
return relay(cls)(1)
class D(C): pass
reveal_type(D.make())it appears that this change causes a regression here; trunk accepts this, as do other type checkers, but with this diff we get
INFO typevar-new.py:11:20-32: revealed type: (x: int) -> C [reveal-type]
ERROR typevar-new.py:12:16-29: Returned type `C` is not assignable to declared return type `Self@C` [bad-return]
INFO typevar-new.py:20:20-32: revealed type: (x: int) -> Legacy [reveal-type]
ERROR typevar-new.py:21:16-29: Returned type `Legacy` is not assignable to declared return type `Self@Legacy` [bad-return]
INFO typevar-new.py:24:12-22: revealed type: D [reveal-type]
INFO 2 errors
We need to work out why the current adapter causes this; I suspect we're making too many assumptions about Self here to handle other ways of annotating cls(?)
An AI reviewer suggests suggests this as a potential fix: expand solved constructor return variables before the compatibility check, then add regression tests.
Converting type[Self], such as 'cls' in a classmethod, to a callable fell back to 'type.__call__', '(*args: Any, **kwds: Any) -> Any'. Passing 'cls' to a ParamSpec-generic function lost its signature, and assigning it to an incompatible callable type was allowed. Convert type[Self] from the constructor of the class bound to Self. As in a call through type[Self], change a constructor return that is an instance of the class to Self, so the converted callable still returns the subclass when the classmethod is inherited. Fixes pyrefly-org#5591
d864692 to
905e2d5
Compare
stroxler
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
|
This pull request has been merged in 70dcfe7. |
Summary
Converting
type[Self], such asclsin a classmethod, ortype[T]for a type variableTbound to a class, to a callable fell back totype.__call__,(*args: Any, **kwds: Any) -> Any. Passingclsto a ParamSpec-generic function lost its signature, and assigning it to an incompatible callable type was allowed.Convert
type[Self]andtype[T]from the constructor of the class they are bound to. As in a call through them, change a constructor return that is an instance of the class toSelforT, so the converted callable still returns the subclass when the classmethod is inherited. Expand the solvedTof__new__(cls: type[T]) -> Tbefore checking whether its return is an instance of the class.Fixes meta#5591
Test Plan
cargo test -p pyrefly --lib test::constructors::test_type_of_self_as_callable