From 4616fa23246d28f435d1abb269670702ebf755b9 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 2 Sep 2023 01:47:46 -0700 Subject: [PATCH 1/7] Believe arbitrary __new__ return types Fixes https://github.com/python/mypy/issues/1020#issuecomment-1318657466 Surprisingly popular comment on a closed issue. We still issue the warning, but we do trust the return type instead of overruling it. Maybe fixes #16012 --- mypy/typeops.py | 4 ++-- test-data/unit/check-classes.test | 4 ++-- test-data/unit/check-enum.test | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mypy/typeops.py b/mypy/typeops.py index f9c1914cc9a8..ad7ca1f18f1b 100644 --- a/mypy/typeops.py +++ b/mypy/typeops.py @@ -197,9 +197,9 @@ def class_callable( # by accident. Like `Hashable` is a subtype of `object`. See #11799 and isinstance(default_ret_type, Instance) and not default_ret_type.type.is_protocol - # Only use the declared return type from __new__ or declared self in __init__ + # Only use the declared return type from declared self in __init__ # if it is actually returning a subtype of what we would return otherwise. - and is_subtype(explicit_type, default_ret_type, ignore_type_params=True) + and (is_new or is_subtype(explicit_type, default_ret_type, ignore_type_params=True)) ): ret_type: Type = explicit_type else: diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 4bc1e50f7be9..8babe401afb3 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -6883,7 +6883,7 @@ class A: def __new__(cls) -> int: # E: Incompatible return type for "__new__" (returns "int", but must return a subtype of "A") pass -reveal_type(A()) # N: Revealed type is "__main__.A" +reveal_type(A()) # N: Revealed type is "builtins.int" [case testNewReturnType4] from typing import TypeVar, Type @@ -6960,7 +6960,7 @@ class A: class B(A): pass -reveal_type(B()) # N: Revealed type is "__main__.B" +reveal_type(B()) # N: Revealed type is "__main__.A" [case testNewReturnType10] # https://github.com/python/mypy/issues/11398 diff --git a/test-data/unit/check-enum.test b/test-data/unit/check-enum.test index 6779ae266454..307b26cba483 100644 --- a/test-data/unit/check-enum.test +++ b/test-data/unit/check-enum.test @@ -1360,7 +1360,7 @@ class Bar(Foo): A = 1 B = 2 -a = Bar.A +a = Bar.A # E: "Type[Foo]" has no attribute "A" reveal_type(a.value) # N: Revealed type is "Any" reveal_type(a._value_) # N: Revealed type is "Any" [builtins fixtures/primitives.pyi] From e2d3a5e54d6eb9f9c7058f19b294fa6a6a5761a8 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 2 Sep 2023 02:29:31 -0700 Subject: [PATCH 2/7] changes --- mypy/typeops.py | 34 +++++++++++++++++++------------ test-data/unit/check-classes.test | 19 ++++++++++++++--- test-data/unit/check-enum.test | 2 +- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/mypy/typeops.py b/mypy/typeops.py index ad7ca1f18f1b..25cbab0b084b 100644 --- a/mypy/typeops.py +++ b/mypy/typeops.py @@ -191,19 +191,27 @@ def class_callable( orig_self_type = get_proper_type(orig_self_type) default_ret_type = fill_typevars(info) explicit_type = init_ret_type if is_new else orig_self_type - if ( - isinstance(explicit_type, (Instance, TupleType, UninhabitedType)) - # We have to skip protocols, because it can be a subtype of a return type - # by accident. Like `Hashable` is a subtype of `object`. See #11799 - and isinstance(default_ret_type, Instance) - and not default_ret_type.type.is_protocol - # Only use the declared return type from declared self in __init__ - # if it is actually returning a subtype of what we would return otherwise. - and (is_new or is_subtype(explicit_type, default_ret_type, ignore_type_params=True)) - ): - ret_type: Type = explicit_type - else: - ret_type = default_ret_type + + ret_type: Type = default_ret_type + if isinstance(explicit_type, (Instance, TupleType, UninhabitedType)): + if is_new: + # For a long time, mypy didn't truly believe annotations on __new__ + # This has led to some proliferation of code that depends on this, namely + # annotations on __new__ that hardcode the class in the return type. + # This can then cause problems for subclasses. Preserve the old behaviour in + # this case (although we should probably change it at some point) + if not is_subtype(default_ret_type, explicit_type, ignore_type_params=True): + ret_type = explicit_type + elif ( + # We have to skip protocols, because it can be a subtype of a return type + # by accident. Like `Hashable` is a subtype of `object`. See #11799 + isinstance(default_ret_type, Instance) + and not default_ret_type.type.is_protocol + # Only use the declared return type from declared self in __init__ + # if it is actually returning a subtype of what we would return otherwise. + and is_subtype(explicit_type, default_ret_type, ignore_type_params=True) + ): + ret_type = explicit_type callable_type = init_type.copy_modified( ret_type=ret_type, diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index 8babe401afb3..bae4e9ed2a26 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -6913,8 +6913,8 @@ class O(Generic[T]): def __new__(cls, x: int = 0) -> O[Any]: pass -reveal_type(O()) # N: Revealed type is "__main__.O[builtins.int]" -reveal_type(O(10)) # N: Revealed type is "__main__.O[builtins.str]" +reveal_type(O()) # N: Revealed type is "__main__.O[Never]" +reveal_type(O(10)) # N: Revealed type is "__main__.O[Never]" [case testNewReturnType6] from typing import Tuple, Optional @@ -6960,7 +6960,7 @@ class A: class B(A): pass -reveal_type(B()) # N: Revealed type is "__main__.A" +reveal_type(B()) # N: Revealed type is "__main__.B" [case testNewReturnType10] # https://github.com/python/mypy/issues/11398 @@ -6993,6 +6993,19 @@ class MyMetaClass(type): class MyClass(metaclass=MyMetaClass): pass +[case testNewReturnType13] +from typing import Protocol + +class Foo(Protocol): + def foo(self) -> str: ... + +class A: + def __new__(cls) -> Foo: ... # E: Incompatible return type for "__new__" (returns "Foo", but must return a subtype of "A") + +reveal_type(A()) # E: Cannot instantiate protocol class "Foo" \ + # N: Revealed type is "__main__.Foo" +reveal_type(A().foo()) # E: Cannot instantiate protocol class "Foo" \ + # N: Revealed type is "builtins.str" [case testMetaclassPlaceholderNode] from sympy.assumptions import ManagedProperties diff --git a/test-data/unit/check-enum.test b/test-data/unit/check-enum.test index 307b26cba483..6779ae266454 100644 --- a/test-data/unit/check-enum.test +++ b/test-data/unit/check-enum.test @@ -1360,7 +1360,7 @@ class Bar(Foo): A = 1 B = 2 -a = Bar.A # E: "Type[Foo]" has no attribute "A" +a = Bar.A reveal_type(a.value) # N: Revealed type is "Any" reveal_type(a._value_) # N: Revealed type is "Any" [builtins fixtures/primitives.pyi] From 2bd91d557f15365aabde1e09a30a31a0d275f28b Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sat, 2 Sep 2023 02:53:05 -0700 Subject: [PATCH 3/7] more changes --- mypy/typeops.py | 7 ++++++- test-data/unit/check-classes.test | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/mypy/typeops.py b/mypy/typeops.py index 25cbab0b084b..7b938b68f970 100644 --- a/mypy/typeops.py +++ b/mypy/typeops.py @@ -200,7 +200,12 @@ def class_callable( # annotations on __new__ that hardcode the class in the return type. # This can then cause problems for subclasses. Preserve the old behaviour in # this case (although we should probably change it at some point) - if not is_subtype(default_ret_type, explicit_type, ignore_type_params=True): + # See testValueTypeWithNewInParentClass + # Also see testSelfTypeInGenericClassUsedFromAnotherGenericClass1 + if ( + not is_subtype(default_ret_type, explicit_type, ignore_type_params=True) + or is_subtype(explicit_type, default_ret_type, ignore_type_params=True) + ): ret_type = explicit_type elif ( # We have to skip protocols, because it can be a subtype of a return type diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index bae4e9ed2a26..ba7aa17d25ca 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -6913,8 +6913,8 @@ class O(Generic[T]): def __new__(cls, x: int = 0) -> O[Any]: pass -reveal_type(O()) # N: Revealed type is "__main__.O[Never]" -reveal_type(O(10)) # N: Revealed type is "__main__.O[Never]" +reveal_type(O()) # N: Revealed type is "__main__.O[builtins.int]" +reveal_type(O(10)) # N: Revealed type is "__main__.O[builtins.str]" [case testNewReturnType6] from typing import Tuple, Optional From 393b303bf69bc68ce917d844840ac4fa8af91905 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sun, 3 Sep 2023 17:41:50 -0700 Subject: [PATCH 4/7] hacks --- mypy/checkexpr.py | 10 ++++----- mypy/checkmember.py | 27 ++++++++++++++++++++++-- mypy/typeops.py | 7 +++---- mypy/types.py | 2 +- test-data/unit/check-classes.test | 35 +++++++++++++++++++++++++++++++ 5 files changed, 69 insertions(+), 12 deletions(-) diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py index 22a9852545b7..95cc56dd545d 100644 --- a/mypy/checkexpr.py +++ b/mypy/checkexpr.py @@ -3182,11 +3182,11 @@ def analyze_ordinary_member_access(self, e: MemberExpr, is_lvalue: bool) -> Type member_type = analyze_member_access( e.name, original_type, - e, - is_lvalue, - False, - False, - self.msg, + context=e, + is_lvalue=is_lvalue, + is_super=False, + is_operator=False, + msg=self.msg, original_type=original_type, chk=self.chk, in_literal_context=self.is_literal_context(), diff --git a/mypy/checkmember.py b/mypy/checkmember.py index f7d002f17eb9..252415727bef 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -371,8 +371,27 @@ def validate_super_call(node: FuncBase, mx: MemberContext) -> None: def analyze_type_callable_member_access(name: str, typ: FunctionLike, mx: MemberContext) -> Type: # Class attribute. # TODO super? - ret_type = typ.items[0].ret_type + item = typ.items[0] + ret_type = item.ret_type assert isinstance(ret_type, ProperType) + + # The following is a hack. mypy often represents types as CallableType, where the signature of + # CallableType is determined by __new__ or __init__ of the type (this logic is in + # type_object_type). Then if we ever need the TypeInfo or an instance of the type, we fish + # around for the return type in CallableType.type_object. Unfortunately, this is incorrect if + # __new__ returns an unrelated type, but we can kind of salvage things by fishing around in + # CallableType.bound_args + self_type: Type = typ + if len(item.bound_args) == 1 and item.bound_args[0]: + bound_arg = get_proper_type(item.bound_args[0]) + if isinstance(bound_arg, Instance) and isinstance(ret_type, Instance): + # Unfortunately, generic arguments have already been determined for us. We need these, + # see e.g. testGenericClassMethodUnboundOnClass. So just copy them over to our type. + # This does the wrong thing with custom __new__, see testNewReturnType15, but is + # a lesser evil. + ret_type = bound_arg.copy_modified(args=ret_type.args) + self_type = TypeType(ret_type) + if isinstance(ret_type, TupleType): ret_type = tuple_fallback(ret_type) if isinstance(ret_type, TypedDictType): @@ -394,7 +413,11 @@ def analyze_type_callable_member_access(name: str, typ: FunctionLike, mx: Member # See https://github.com/python/mypy/pull/1787 for more info. # TODO: do not rely on same type variables being present in all constructor overloads. result = analyze_class_attribute_access( - ret_type, name, mx, original_vars=typ.items[0].variables, mcs_fallback=typ.fallback + ret_type, + name, + mx.copy_modified(self_type=self_type), + original_vars=typ.items[0].variables, + mcs_fallback=typ.fallback, ) if result: return result diff --git a/mypy/typeops.py b/mypy/typeops.py index 7b938b68f970..28ff8dd04734 100644 --- a/mypy/typeops.py +++ b/mypy/typeops.py @@ -202,10 +202,9 @@ def class_callable( # this case (although we should probably change it at some point) # See testValueTypeWithNewInParentClass # Also see testSelfTypeInGenericClassUsedFromAnotherGenericClass1 - if ( - not is_subtype(default_ret_type, explicit_type, ignore_type_params=True) - or is_subtype(explicit_type, default_ret_type, ignore_type_params=True) - ): + if not is_subtype( + default_ret_type, explicit_type, ignore_type_params=True + ) or is_subtype(explicit_type, default_ret_type, ignore_type_params=True): ret_type = explicit_type elif ( # We have to skip protocols, because it can be a subtype of a return type diff --git a/mypy/types.py b/mypy/types.py index f974157ce84d..9c51e452a3ff 100644 --- a/mypy/types.py +++ b/mypy/types.py @@ -1466,7 +1466,7 @@ def deserialize(cls, data: JsonDict | str) -> Instance: def copy_modified( self, *, - args: Bogus[list[Type]] = _dummy, + args: Bogus[Sequence[Type]] = _dummy, last_known_value: Bogus[LiteralType | None] = _dummy, ) -> Instance: new = Instance( diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index ba7aa17d25ca..ec6c3ef61a60 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -7007,6 +7007,41 @@ reveal_type(A()) # E: Cannot instantiate protocol class "Foo" \ reveal_type(A().foo()) # E: Cannot instantiate protocol class "Foo" \ # N: Revealed type is "builtins.str" +[case testNewReturnType14] +from __future__ import annotations + +class A: + def __new__(cls) -> int: raise # E: Incompatible return type for "__new__" (returns "int", but must return a subtype of "A") + +class B(A): + @classmethod + def foo(cls) -> int: raise + +reveal_type(B.foo()) # N: Revealed type is "builtins.int" +[builtins fixtures/classmethod.pyi] + +[case testNewReturnType15] +from typing import Generic, Type, TypeVar + +T = TypeVar("T") + +class A(Generic[T]): + def __new__(cls) -> B[int]: ... + @classmethod + def foo(cls: Type[T]) -> T: ... + +class B(A[T]): ... + +# These are incorrect, should be Any and str +# See https://github.com/python/mypy/pull/16020 +reveal_type(B.foo()) # N: Revealed type is "builtins.int" +reveal_type(B[str].foo()) # N: Revealed type is "builtins.int" + +class C(A[str]): ... + +reveal_type(C.foo()) # N: Revealed type is "builtins.str" +[builtins fixtures/classmethod.pyi] + [case testMetaclassPlaceholderNode] from sympy.assumptions import ManagedProperties from sympy.ops import AssocOp From 7b7d76dc8286242c23fb63589991127989c6602a Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Sun, 3 Sep 2023 18:58:25 -0700 Subject: [PATCH 5/7] fix instantiate --- mypy/typeops.py | 3 +++ test-data/unit/check-classes.test | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/mypy/typeops.py b/mypy/typeops.py index 28ff8dd04734..03e3cabbbbd1 100644 --- a/mypy/typeops.py +++ b/mypy/typeops.py @@ -193,6 +193,7 @@ def class_callable( explicit_type = init_ret_type if is_new else orig_self_type ret_type: Type = default_ret_type + from_type_type = init_type.from_type_type if isinstance(explicit_type, (Instance, TupleType, UninhabitedType)): if is_new: # For a long time, mypy didn't truly believe annotations on __new__ @@ -206,6 +207,7 @@ def class_callable( default_ret_type, explicit_type, ignore_type_params=True ) or is_subtype(explicit_type, default_ret_type, ignore_type_params=True): ret_type = explicit_type + from_type_type = True elif ( # We have to skip protocols, because it can be a subtype of a return type # by accident. Like `Hashable` is a subtype of `object`. See #11799 @@ -223,6 +225,7 @@ def class_callable( name=None, variables=variables, special_sig=special_sig, + from_type_type=from_type_type, ) c = callable_type.with_name(info.name) return c diff --git a/test-data/unit/check-classes.test b/test-data/unit/check-classes.test index ec6c3ef61a60..7b6130048dac 100644 --- a/test-data/unit/check-classes.test +++ b/test-data/unit/check-classes.test @@ -7002,10 +7002,10 @@ class Foo(Protocol): class A: def __new__(cls) -> Foo: ... # E: Incompatible return type for "__new__" (returns "Foo", but must return a subtype of "A") -reveal_type(A()) # E: Cannot instantiate protocol class "Foo" \ - # N: Revealed type is "__main__.Foo" -reveal_type(A().foo()) # E: Cannot instantiate protocol class "Foo" \ - # N: Revealed type is "builtins.str" +reveal_type(A()) # N: Revealed type is "__main__.Foo" +reveal_type(A().foo()) # N: Revealed type is "builtins.str" + + [case testNewReturnType14] from __future__ import annotations From aa5df74a2e35d6ed44d18288a1580333e7396fdc Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Mon, 4 Sep 2023 14:54:03 -0700 Subject: [PATCH 6/7] clarify comment --- mypy/checkmember.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 252415727bef..025d3bd1a798 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -388,7 +388,7 @@ def analyze_type_callable_member_access(name: str, typ: FunctionLike, mx: Member # Unfortunately, generic arguments have already been determined for us. We need these, # see e.g. testGenericClassMethodUnboundOnClass. So just copy them over to our type. # This does the wrong thing with custom __new__, see testNewReturnType15, but is - # a lesser evil. + # no worse than previous behaviour. ret_type = bound_arg.copy_modified(args=ret_type.args) self_type = TypeType(ret_type) From 59a5aee40b727952c02242e868ebebd4042dd0a6 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Mon, 4 Sep 2023 18:35:09 -0700 Subject: [PATCH 7/7] clarify comment --- mypy/checkmember.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/checkmember.py b/mypy/checkmember.py index 025d3bd1a798..96ccd3dde286 100644 --- a/mypy/checkmember.py +++ b/mypy/checkmember.py @@ -378,8 +378,8 @@ def analyze_type_callable_member_access(name: str, typ: FunctionLike, mx: Member # The following is a hack. mypy often represents types as CallableType, where the signature of # CallableType is determined by __new__ or __init__ of the type (this logic is in # type_object_type). Then if we ever need the TypeInfo or an instance of the type, we fish - # around for the return type in CallableType.type_object. Unfortunately, this is incorrect if - # __new__ returns an unrelated type, but we can kind of salvage things by fishing around in + # around for the return type, e.g. in CallableType.type_object. Unfortunately, this is incorrect + # if __new__ returns an unrelated type, but we can kind of salvage things here using # CallableType.bound_args self_type: Type = typ if len(item.bound_args) == 1 and item.bound_args[0]: