Mypy: warn_no_return = False: handle implicit "return None" (not ignoring return type)

Created on 14 Sep 2019  路  15Comments  路  Source: python/mypy

warn_no_return is on by default, but does not take into account that None is returned explicitly.

I think it should maybe only warn if the return type does not include None (i.e. uses typing.Optional).

Given t_mypy.py:

import typing


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


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


a1 = no_return_optional(True)
a2 = no_return_optional(False)
reveal_type(a1)
reveal_type(a2)

b1 = no_return(True)
b2 = no_return(False)
reveal_type(b1)
reveal_type(b2)

mypy --warn-no-return shows:

t_mypy.py:4: error: Missing return statement
t_mypy.py:9: error: Missing return statement
t_mypy.py:16: note: Revealed type is 'Union[builtins.str, None]'
t_mypy.py:17: note: Revealed type is 'Union[builtins.str, None]'
t_mypy.py:21: note: Revealed type is 'builtins.str'
t_mypy.py:22: note: Revealed type is 'builtins.str'
Found 2 errors in 1 file (checked 1 source file)

mypy --no-warn-no-return does not show any error, but should warn/error about the missing return statement / wrong return value for no_return (which is annotated to return str, but might return None).

mypy 0.730+dev.28423cd78062b81365ec9aaa591b4e823af1606d

documentation needs discussion topic-usability

Most helpful comment

Ok, fair enough - I do not want to waste anyone's time of course.
But just to be clear - the option is not named ignore_errors_with_no_return, i.e. it is not clear but surprising that it behaves that way.
I also think that mypy ignoring the fact that Python has implicit returns is a design decision to be revisited, and that should be done before adding a warning to the docs then probably.

All 15 comments

This is by design. warn_no_return excepts an explicit return statement even in functions that return an optional type. The assumption is that relying on the implicit None return is often an error (or at least a code smell).

When the check is disabled, mypy effectively behaves as if there Python had no implicit None return. I can see how this can be surprising, and I may be persuaded that this is not the best way to implement this.

At the very least we could make the documentation more detailed.

When the check is disabled, mypy effectively behaves as if there Python had
no implicit None return. I can see how this can be surprising, and I may be
persuaded that this is not the best way to implement this.

The main problem I see here is that you are forced to use the default
(warn_no_return = True), since otherwise you are not getting an error with:

def f(a) -> int:
    if a:
        return 1

Disabling the warning is useful/necessary to not change code unnecessarily
when adding type annotations, but currently you are forced to add return None at the end always despite of the setting to not have errors go silent
(btw: return by itself results in "Return value expected").

Therefore I think it would be great if the behavior in this regard could be
changed (as if return None was used), since otherwise disabling the warning
also hides real errors silently.
This would affect the non-default part of the option.

While I'd still prefer to have Optional[鈥 not cause a warning without an explicit return None by default, I think the above is more important for now, and have changed/clarified the issue title in this regard.

Docs for reference: https://mypy.readthedocs.io/en/latest/command_line.html#cmdoption-mypy-no-warn-no-return

FWIW, I am fine with the current behaviour -- "explicit is better than implicit". The error message and the docs however can be improved.

@ilevkivskyi
Don't you agree that mypy not showing an error with the above example (with warn_no_return = False) is wrong / hiding the error?

I don't see a problem if someone opts-in to unsafety and gets an unsafety. (Also this is my final opinion here, I don't have time to write an essay about why/what/etc.)

Ok, fair enough - I do not want to waste anyone's time of course.
But just to be clear - the option is not named ignore_errors_with_no_return, i.e. it is not clear but surprising that it behaves that way.
I also think that mypy ignoring the fact that Python has implicit returns is a design decision to be revisited, and that should be done before adding a warning to the docs then probably.

I also think that mypy ignoring the fact that Python has implicit returns is a design decision to be revisited

I agree wholeheartedly. From https://github.com/python/mypy/issues/3974#issuecomment-331648044:

This is a style issue

Indeed, Mypy is overreaching here and enforcing a stylistic choice at the cost of correct language semantics.

This is the only reason I'm not using Mypy 馃槩

The most likely way around this that I can see is adding support for enabling/disabling individual error codes, and giving the relevant error message a specific error code that can be disabled (which would hide all these messages). Now anybody who doesn't like this error can disable the relevant error code.

I'm happy to accept a PR that implements enabling/disabling error codes (we should first discuss how this would be configured in a separate issue).

There's actually a few scenarios relevant here:

Scenario 1

def f1(a) -> int:
    if a:
        return 1
  • Currently, this fails with error: Missing return statement.
  • This makes sense, the function is missing a return.

Scenario 2

def f2(a) -> Optional[int]:
    if a:
        return 1
  • Currently, this fails with: error: Missing return statement.
  • This code is fine, it explicitly states that it returns None.

Scenario 3

def f3(a) -> Optional[int]:
    if a:
        return 1
    return None
  • This one is doubly explicit, and passes fine.

Scenario 4

def f4(a) -> None:
    pass
  • This looks like scenario 2 (None returned implicitly).
  • However, this one passes checks fine.
  • Scenario 4 is because of a mypy exception for functions with a one-line body. These are used as stubs and we found usability issues around this in real-world code. Arguably this behavior should not be the default (except for stub files), and we could introduce a flag to enable it.

  • The error you get in Scenario 2 enforces a PEP 8 rule that is dear to my heart -- if you return a value, you must explicitly return a value on all code paths. Even if that value has an Optional type. The quickest way to avoid the error is to add return None at the end. If you don't like that, maybe use --no-warn-no-return.

My issue here is that I have to be explicit twice. I'm more of the DRY mentality:

Initial code:

def f2(a) -> int:
    if a:
        return 1

Be explicit about the return value possibly being None:

def f2(a) -> Optional[int]:
    if a:
        return 1

Be explicit about the return value possibly being None again:

def f2(a) -> Optional[int]:
    if a:
        return 1
    return None

It's very annoying that 1/3 of my function is now just redundant code that does nothing. It just hurts readability. It was obvious from the start it could return None. I can respect having to be explicit about it by adding Optional. Having to be explicit about it twice it just noise.

Whatever.

Note there is now support for enabling/disabling error codes https://github.com/python/mypy/pull/9172 which will be released soon https://github.com/python/mypy/issues/9290

I'm not sure what this code would be though

While I agree in general with the Zen of Python,
I feel that warning in this particular case represents feature creep for mypy.

Specifically, mypy is a type checker, and should (in my opinion) allow whatever silly, broken or ugly code someone may write, as long as types are correct. It's a job for other tools to enforce PEP-8 or local style conventions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

co-dh picture co-dh  路  3Comments

jstasiak picture jstasiak  路  3Comments

Stiivi picture Stiivi  路  3Comments

gregbedwell picture gregbedwell  路  3Comments

PeterJCLaw picture PeterJCLaw  路  3Comments