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

Stub functions should only typecheck for None #4066

Closed
Wilfred opened this issue Oct 6, 2017 · 14 comments
Closed

Stub functions should only typecheck for None #4066

Wilfred opened this issue Oct 6, 2017 · 14 comments

Comments

@Wilfred
Copy link

Wilfred commented Oct 6, 2017

This passes in mypy 0.521:

def foo(): # type: () -> int
    pass

mypy --py2 --strict demo.py does not show any errors. If a function body does not contain a return statement or a yield statement it should only have a type of None I believe.

@ilevkivskyi
Copy link
Member

This is a special case in mypy. If a function have an empty body then it can have arbitrary return type. This is useful for testing, and is necessary for stubs. We have lots of tests that use this, so we can't disable this in non-stub files. Alternatively we can somehow detect whether we are running tests, and display an error otherwise.

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 6, 2017

Mypy should probably eventually enforce this outside stubs. One way would be to make this an option (which doesn't need to be shown in help, perhaps) and turn it on by default, but we'd turn it off for most tests. Detecting stubs is easy, so it won't be a problem. We don't need to actually detect tests if all test runners just explicitly toggle the option.

@ilevkivskyi
Copy link
Member

@JukkaL

We don't need to actually detect tests if all test runners just explicitly toggle the option.

Yes, I was thinking about the same idea.

@ilevkivskyi
Copy link
Member

See also #2350

@Wilfred
Copy link
Author

Wilfred commented Oct 9, 2017

Aha, I didn't realise that mypy treated stubs specially. My intent was to minimise my example, but I initially encountered this issue on a function that calls exit:

import sys

def fatal_error(message):
    # type: (str) -> dict
    print "Oh no! {}".format(message)
    sys.exit(1)

I was expecting mypy to give a type error here.

@JelleZijlstra
Copy link
Member

That's expected: that function basically returns NoReturn, and NoReturn is a subtype of every type.

@gvanrossum
Copy link
Member

I propose to change the title of this issue to "Get rid of exception for functions with just pass as body" -- or something better (but I can't think of something better). The current title is too wide and in general incorrect.

@Wilfred Wilfred changed the title A function without return or yield should only typecheck for None Stub functions should only typecheck for None Oct 10, 2017
@Wilfred
Copy link
Author

Wilfred commented Oct 10, 2017

I've updated the issue title.

@Wilfred
Copy link
Author

Wilfred commented Oct 10, 2017

@JelleZijlstra the NoReturn behaviour was surprising to me. Could I open an issue for a --strict option that disables this behaviour, or is this working as intended?

@ilevkivskyi
Copy link
Member

ilevkivskyi commented Oct 10, 2017

or is this working as intended?

This is intended. It is OK for a function to declare a return type wider than actual (NoReturn in this case).

@Wilfred
Copy link
Author

Wilfred commented Oct 12, 2017

@JelleZijlstra out of interest, what's the benefit of that? When is NoReturn being a subtype of everything useful?

@ilevkivskyi
Copy link
Member

@Wilfred NoReturn is the bottom type, i.e. a subtype of every type, so I would put this vice versa, what's the benefit of complicating the type system by not doing that?

@ilevkivskyi
Copy link
Member

@Wilfred To clarify, this was a rhetorical question, this discussion is off-topic for this issue, if you want we can continue it elsewhere.

@ilevkivskyi
Copy link
Member

I think this is just a duplicate of #2350

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

5 participants