Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Believe more __new__ return types #16020

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
27 changes: 25 additions & 2 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, 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]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with other comments that using bound_args doesn't look like something we want to do. I think storing the real self type in a new attribute might be a better approach. All type operations should be updated to do something reasonable with the attribute.

It seems like the above should also fix the issue that no error is currently reported here, and perhaps other related issues:

class C:
    def __new__(self) -> D:
        return D()

class D(C):
    x: int

C.x  # No Error, but fails at runtime

We'd use use the new attribute when looking up class attributes.

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
# no worse than previous behaviour.
ret_type = bound_arg.copy_modified(args=ret_type.args)
Copy link
Contributor

@ikonst ikonst Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can

Suggested change
ret_type = bound_arg.copy_modified(args=ret_type.args)
ret_type = bound_arg.copy_modified()

if you change checkexpr.py's apply_generic_arguments to expand bound_args (not sure if it's correct to do...):

 return callable.copy_modified(
     ret_type=expand_type(callable.ret_type, id_to_type),
+    bound_args=[
+        expand_type(ba, id_to_type) if ba is not None else None
+        for ba in callable.bound_args
+    ],
     variables=remaining_tvars,
     type_guard=type_guard,
 )

self_type = TypeType(ret_type)

if isinstance(ret_type, TupleType):
ret_type = tuple_fallback(ret_type)
if isinstance(ret_type, TypedDictType):
Expand All @@ -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
Expand Down
41 changes: 28 additions & 13 deletions mypy/typeops.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,26 +191,41 @@ 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 __new__ or 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: Type = explicit_type
else:
ret_type = default_ret_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__
# 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)
# 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
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
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,
fallback=type_type,
name=None,
variables=variables,
special_sig=special_sig,
from_type_type=from_type_type,
)
c = callable_type.with_name(info.name)
return c
Expand Down
2 changes: 1 addition & 1 deletion mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
50 changes: 49 additions & 1 deletion test-data/unit/check-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -6993,6 +6993,54 @@ 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()) # N: Revealed type is "__main__.Foo"
reveal_type(A().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
Expand Down