Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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
41 changes: 18 additions & 23 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -4895,6 +4895,10 @@ def infer_context_dependent(
return typ

def check_return_stmt(self, s: ReturnStmt) -> None:

def is_notimplemented(t: object) -> bool:
return isinstance(t, Instance) and t.type.fullname == "builtins._NotImplementedType"

defn = self.scope.current_function()
if defn is not None:
if defn.is_generator:
Expand Down Expand Up @@ -4942,17 +4946,11 @@ def check_return_stmt(self, s: ReturnStmt) -> None:
s.expr, return_type, allow_none_return=allow_none_func_call
)
)
# Treat NotImplemented as having type Any, consistent with its
# definition in typeshed prior to python/typeshed#4222.
if (
isinstance(typ, Instance)
and typ.type.fullname == "builtins._NotImplementedType"
):
typ = AnyType(TypeOfAny.special_form)

if defn.is_async_generator:
self.fail(message_registry.RETURN_IN_ASYNC_GENERATOR, s)
return

# Returning a value of type Any is always fine.
if isinstance(typ, AnyType):
# (Unless you asked to be warned in that case, and the
Expand All @@ -4961,10 +4959,6 @@ def check_return_stmt(self, s: ReturnStmt) -> None:
self.options.warn_return_any
and not self.current_node_deferred
and not is_proper_subtype(AnyType(TypeOfAny.special_form), return_type)
and not (
defn.name in BINARY_MAGIC_METHODS
and is_literal_not_implemented(s.expr)
)
and not (
isinstance(return_type, Instance)
and return_type.type.fullname == "builtins.object"
Expand All @@ -4983,9 +4977,17 @@ def check_return_stmt(self, s: ReturnStmt) -> None:
return
self.fail(message_registry.NO_RETURN_VALUE_EXPECTED, s)
else:
typ_: Type = typ
if defn.name in BINARY_MAGIC_METHODS or defn.name == "__subclasshook__":
if is_notimplemented(typ):
return
if isinstance(typ, UnionType):
typ_ = UnionType.make_union(
[i for i in typ.items if not is_notimplemented(i)]
)
self.check_subtype(
subtype_label="got",
subtype=typ,
subtype=typ_,
supertype_label="expected",
supertype=return_type,
context=s.expr,
Expand Down Expand Up @@ -5098,22 +5100,15 @@ def type_check_raise(self, e: Expression, s: RaiseStmt, optional: bool = False)
# where we allow `raise e from None`.
expected_type_items.append(NoneType())

self.check_subtype(
typ, UnionType.make_union(expected_type_items), s, message_registry.INVALID_EXCEPTION
)
message = message_registry.INVALID_EXCEPTION
if isinstance(typ, Instance) and typ.type.fullname == "builtins._NotImplementedType":
message = message.with_additional_msg('; did you mean "NotImplementedError"?')
self.check_subtype(typ, UnionType.make_union(expected_type_items), s, message)

if isinstance(typ, FunctionLike):
# https://github.com/python/mypy/issues/11089
self.expr_checker.check_call(typ, [], [], e)

if isinstance(typ, Instance) and typ.type.fullname == "builtins._NotImplementedType":
self.fail(
message_registry.INVALID_EXCEPTION.with_additional_msg(
'; did you mean "NotImplementedError"?'
),
s,
)

def visit_try_stmt(self, s: TryStmt) -> None:
"""Type check a try statement."""

Expand Down
8 changes: 6 additions & 2 deletions mypy/mro.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ def calculate_mro(info: TypeInfo, obj_type: Callable[[], Instance] | None = None
mro = linearize_hierarchy(info, obj_type)
assert mro, f"Could not produce a MRO at all for {info}"
info.mro = mro
# The property of falling back to Any is inherited.
info.fallback_to_any = any(baseinfo.fallback_to_any for baseinfo in info.mro)
# The property of falling back to Any is (usually) inherited.
if info.fullname == "builtins._NotImplementedType":
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a nice duct tape application, but maybe we should consider changing that in typeshed (or our own typeshed patch) directly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, my favourite would be to update typeshed. However, this was closed in typeshed/13488 due to something with __eq__ and the required adjustments for type checkers. I will ask around.

I have not modified Mypy's own typeshed patch so far. I will try it...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

...it worked!

info.fallback_to_any = False
else:
info.fallback_to_any = any(baseinfo.fallback_to_any for baseinfo in info.mro)

type_state.reset_all_subtype_caches_for(info)


Expand Down
35 changes: 35 additions & 0 deletions test-data/unit/check-overloading.test
Original file line number Diff line number Diff line change
Expand Up @@ -6852,3 +6852,38 @@ if isinstance(headers, dict):

reveal_type(headers) # N: Revealed type is "Union[__main__.Headers, typing.Iterable[tuple[builtins.bytes, builtins.bytes]]]"
[builtins fixtures/isinstancelist.pyi]

[case testReturnNotImplementedInBinaryMagicMethods]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you move this to check-overloading?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These tests were originally in the "Returning Any" section of check-warnings.test, but that section no longer fits. Since returning NotImplemented is usually applied in the context of operator overloading, I moved the tests to check-overloading.test. Do you know a place that fits better?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd probably put them in check-classes.test, but that one is already too big, so I'm fine with your decision - just a bit weird to have a set of tests without a single @overload in an overloads test file:)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it is. But we may add many explicit overloads to them later...

from typing import Union
class A:
def __add__(self, other: object) -> int:
return NotImplemented
def __radd__(self, other: object) -> Union[int, NotImplementedType]:
return NotImplemented
def __sub__(self, other: object) -> Union[int, NotImplementedType]:
return 1
def __isub__(self, other: object) -> int:
x: Union[int, NotImplementedType]
return x
def __mul__(self, other: object) -> Union[int, NotImplementedType]:
x: Union[int, NotImplementedType]
return x

[builtins fixtures/notimplemented.pyi]

[case testReturnNotImplementedABCSubclassHookMethod]
class A:
@classmethod
def __subclasshook__(cls, t: type[object], /) -> bool:
return NotImplemented
[builtins fixtures/notimplemented.pyi]

[case testReturnNotImplementedInNormalMethods]
from typing import Union
class A:
def f(self) -> bool: return NotImplemented # E: Incompatible return value type (got "_NotImplementedType", expected "bool")
def g(self) -> NotImplementedType: return True # E: Incompatible return value type (got "bool", expected "_NotImplementedType")
def h(self) -> NotImplementedType: return NotImplemented
def i(self) -> Union[bool, NotImplementedType]: return NotImplemented
def j(self) -> Union[bool, NotImplementedType]: return True
[builtins fixtures/notimplemented.pyi]
15 changes: 0 additions & 15 deletions test-data/unit/check-warnings.test
Original file line number Diff line number Diff line change
Expand Up @@ -178,21 +178,6 @@ def f() -> int: return g()
[out]
main:4: error: Returning Any from function declared to return "int"

[case testReturnAnyForNotImplementedInBinaryMagicMethods]
# flags: --warn-return-any
class A:
def __eq__(self, other: object) -> bool: return NotImplemented
[builtins fixtures/notimplemented.pyi]
[out]

[case testReturnAnyForNotImplementedInNormalMethods]
# flags: --warn-return-any
class A:
def some(self) -> bool: return NotImplemented
[builtins fixtures/notimplemented.pyi]
[out]
main:3: error: Returning Any from function declared to return "bool"

[case testReturnAnyFromTypedFunctionWithSpecificFormatting]
# flags: --warn-return-any
from typing import Any, Tuple
Expand Down
4 changes: 4 additions & 0 deletions test-data/unit/fixtures/notimplemented.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@ class function: pass
class bool: pass
class int: pass
class str: pass
class tuple: pass
class dict: pass
class classmethod: pass

class _NotImplementedType(Any):
__call__: NotImplemented # type: ignore
NotImplemented: _NotImplementedType

NotImplementedType = _NotImplementedType

class BaseException: pass