Skip to content

--strict-optional Doesn't consider cases missing return #2366

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

Closed
rowillia opened this issue Oct 28, 2016 · 9 comments
Closed

--strict-optional Doesn't consider cases missing return #2366

rowillia opened this issue Oct 28, 2016 · 9 comments

Comments

@rowillia
Copy link
Contributor

rowillia commented Oct 28, 2016

def foo(a: bool) -> str:
    if a:
        return 'Hello world'

Type checks cleanly with --strict-optional but will return None in cases where a is False. This function should have to have a type signature of (bool) -> Optional[str].

I recently came across the --warn-no-return flag in #1748 . Is the intention is to force all code paths in functions to hit a return statement? I know that'd be more in line with PEP 8, but unfortunately I've got tons of code that relies on the absence of a return statement implying return None that I'd like to typecheck.

If this isn't a bug, I would suggest we change this to "--strict-optional should imply --warn-no-return".

@rwbarton
Copy link
Contributor

but unfortunately I've got tons of code that relies on this behavior that I'd like to typecheck

Relies on what behavior?

@rowillia
Copy link
Contributor Author

@rwbarton updated my issue...code that relies on no return statement returning None

@gvanrossum
Copy link
Member

In the meantime you can just pass both flags right?

@rwbarton
Copy link
Contributor

OK. The way --warn-no-return works is effectively that it inserts a return None at the end of your function if it is possible for execution to flow off the end of the function. If None is a legal return value for your function, for example if your function is declared to return None, or Optional[int], etc., then this is no problem, but for a function like your foo, it'll be an error because None does not have type str (under --strict-optional).

--strict-optional implying --warn-no-return is probably not a good default right now because --warn-no-return is brand new, but perhaps it could be in the future. It's easy enough to just pass both flags of course, but the downside of the status quo is that people have to discover the --warn-no-return option, and the lack of this check is reported pretty often as a bug, so it would probably be worth turning it on by default once the kinks have been worked out.

@gvanrossum
Copy link
Member

it [--warn-no-return] would probably be worth turning it on by default once the kinks have been worked out..

Always, or implied by --strict-optional? (Assuming the latter will take much longer to have its kinks worked out.)

@rowillia
Copy link
Contributor Author

rowillia commented Oct 28, 2016

@gvanrossum Totally, but it would be a nice user experience to have them work in tandem since your types aren't really strict about optionals unless we've also asserted that all code paths return values. I agree with @rwbarton that we should iron out the kinks first though 😄

As a side note, it seems like --warn-no-return is fairly noisy - Filed #2370

@rwbarton
Copy link
Contributor

Oops, I forgot how --warn-no-return works. What it actually does is reject implicitly returning None by falling off the end of the function unless the function is declared as returning exactly None. This means it's actually useful without strict optional checking, but it also means your foo won't work even if the return type is changed to Optional[str].

It'd be easy to make it more lenient under strict optional checking; when it was implemented strict optional checking didn't really work well yet (and even mypy itself is not yet using it).

@ddfisher
Copy link
Collaborator

I don't think we should make it more lenient, though -- falling off the end of the function in those cases is still potentially an error, and it seems reasonable it me to make people be explicit. (Just like we don't allow you to write return in a function that returns Optional[int].)

I'd like to see --warn-no-return become the global default. In my experience so far, it's caught a bunch of errors with a very low false positive rate.

@ilevkivskyi
Copy link
Member

Both --strict-optional and --warn-no-return are on by default, so I don't think this is relevant anymore (and also not giving an error with --no-warn-no-return seems right to me).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants