Skip to content

Check implicit None return is valid when using --no-warn-no-return #13219

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

Merged
merged 4 commits into from
Jul 27, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
34 changes: 23 additions & 11 deletions mypy/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str])
self.accept(item.body)
unreachable = self.binder.is_unreachable()

if self.options.warn_no_return and not unreachable:
if not unreachable and not body_is_trivial:
if defn.is_generator or is_named_instance(
self.return_types[-1], "typing.AwaitableGenerator"
):
Expand All @@ -1228,17 +1228,29 @@ def check_func_def(self, defn: FuncItem, typ: CallableType, name: Optional[str])
return_type = self.get_coroutine_return_type(self.return_types[-1])
else:
return_type = self.return_types[-1]

return_type = get_proper_type(return_type)
if not isinstance(return_type, (NoneType, AnyType)) and not body_is_trivial:
# Control flow fell off the end of a function that was
# declared to return a non-None type and is not
# entirely pass/Ellipsis/raise NotImplementedError.
if isinstance(return_type, UninhabitedType):
# This is a NoReturn function
self.fail(message_registry.INVALID_IMPLICIT_RETURN, defn)
else:
self.fail(message_registry.MISSING_RETURN_STATEMENT, defn)

if self.options.warn_no_return:
if not isinstance(return_type, (NoneType, AnyType)):
# Control flow fell off the end of a function that was
# declared to return a non-None type and is not
# entirely pass/Ellipsis/raise NotImplementedError.
if isinstance(return_type, UninhabitedType):
# This is a NoReturn function
self.fail(message_registry.INVALID_IMPLICIT_RETURN, defn)
else:
self.fail(message_registry.MISSING_RETURN_STATEMENT, defn)
else:
# similar to code in check_return_stmt
self.check_subtype(
subtype_label="implicitly returns",
subtype=NoneType(),
supertype_label="expected",
supertype=return_type,
context=defn,
msg=message_registry.INCOMPATIBLE_RETURN_VALUE_TYPE,
code=codes.RETURN_VALUE,
)

self.return_types.pop()

Expand Down
38 changes: 38 additions & 0 deletions test-data/unit/check-flags.test
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,44 @@ async def h() -> NoReturn: # E: Implicit return in function which does not retu
[builtins fixtures/dict.pyi]
[typing fixtures/typing-async.pyi]

[case testNoWarnNoReturn]
Copy link
Member

Choose a reason for hiding this comment

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

Do we need test cases for ... and pass?

Copy link
Collaborator Author

@hauntsaninja hauntsaninja Jul 23, 2022

Choose a reason for hiding this comment

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

There's no change of behaviour here, so covered by existing tests. (Also note that every test would complain about errors in stubs if the trivial body logic breaks — I know because I broke this locally initially)

# flags: --no-warn-no-return --strict-optional
import typing

def implicit_optional_return(arg) -> typing.Optional[str]:
Copy link
Member

Choose a reason for hiding this comment

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

What will happen for a case like:

def implicit_optional_no_return() -> Optional[int]:
    print(1)  # or any other non-primitive body

Copy link
Member

Choose a reason for hiding this comment

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

And what will happen for just -> int?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No error for the first, error for the second Incompatible return value type (implicitly returns "None", expected "int"). Same as the existing test cases, since the if statement has no impact on reachability.

if arg:
return "false"

def unsound_implicit_return(arg) -> str: # E: Incompatible return value type (implicitly returns "None", expected "str")
if arg:
return "false"

def implicit_return_gen(arg) -> typing.Generator[int, None, typing.Optional[str]]:
yield 1

def unsound_implicit_return_gen(arg) -> typing.Generator[int, None, str]: # E: Incompatible return value type (implicitly returns "None", expected "str")
yield 1
[builtins fixtures/dict.pyi]

[case testNoWarnNoReturnNoStrictOptional]
# flags: --no-warn-no-return --no-strict-optional
import typing

def implicit_optional_return(arg) -> typing.Optional[str]:
if arg:
return "false"

def unsound_implicit_return(arg) -> str:
if arg:
return "false"

def implicit_return_gen(arg) -> typing.Generator[int, None, typing.Optional[str]]:
yield 1

def unsound_implicit_return_gen(arg) -> typing.Generator[int, None, str]:
yield 1
[builtins fixtures/dict.pyi]

[case testNoReturnImportFromTyping]
from typing import NoReturn

Expand Down
4 changes: 2 additions & 2 deletions test-data/unit/check-inline-config.test
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ import a
[file a.py]
# mypy: no-warn-no-return

from typing import List
def foo() -> List:
from typing import Optional, List
def foo() -> Optional[List]:
20

[file b.py.2]
Expand Down