Skip to content

sys.exit() is not considered as noreturn #13592

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
socketpair opened this issue Sep 2, 2022 · 17 comments
Closed

sys.exit() is not considered as noreturn #13592

socketpair opened this issue Sep 2, 2022 · 17 comments
Labels

Comments

@socketpair
Copy link

socketpair commented Sep 2, 2022

https://mypy-play.net/?mypy=latest&python=3.10&flags=strict%2Cstrict-equality&gist=e86aa52c05acd36b6c04067cb6f44ce1

import sys

def main() -> int:
    return sys.exit(42)

MyPy finds nothing wrong here.

Anyway, this should be valid:

try:
    sys.exit(42)
except SystemExit:
    print('xxx') # this line IS reachable.

i.e. this function either raises specific exception or noreturn.

@socketpair socketpair added the bug mypy got something wrong label Sep 2, 2022
@AlexWaygood AlexWaygood added feature and removed bug mypy got something wrong labels Sep 2, 2022
@JelleZijlstra
Copy link
Member

Can you clarify what you think the bug is? Mypy is correct when it doesn't produce any errors on your first sample, because a function that never returns is a valid Callable[[], int].

@sobolevn
Copy link
Member

sobolevn commented Sep 2, 2022

Looks like sys.exit is indeed recognised as returning NoReturn: https://mypy-play.net/?mypy=latest&python=3.10&flags=strict%2Cstrict-equality%2Cwarn-unreachable&gist=ef39ea5ba16960f2dd4274349a2c142d

So, it looks fine to me!

@socketpair
Copy link
Author

Using return value of noreturn function is an error, indeed.

@sobolevn
Copy link
Member

sobolevn commented Sep 3, 2022

Feel free to reopen if you have other questions / problems :)

@sobolevn sobolevn closed this as completed Sep 3, 2022
@socketpair
Copy link
Author

@sobolevn please reopen. It's not fixed actually.

@sobolevn sobolevn reopened this Sep 4, 2022
@sobolevn
Copy link
Member

sobolevn commented Sep 4, 2022

Can you please specify what example is not fixed?

@socketpair
Copy link
Author

The first one. Mypy does not see any errors in using value of noreturn function.

@erictraut
Copy link

The first code sample is not a bug. Mypy is correct in not emitting an error here. NoReturn (which is equivalent to the recently-introduced typing.Never type) is a bottom type in the Python type system, and it is is compatible with all other types. A return value of type NoReturn is compatible with the declared return type int.

@socketpair
Copy link
Author

I think, Noreturn must not be compatible with any other type. What the case when it should be compatible ?

@socketpair
Copy link
Author

socketpair commented Sep 5, 2022

Should be def sys.exit(arg: int) -> Never. Does not it ? That is the same as -> NoReturn here. Both must be incompatible with any other type, like int.

@erictraut
Copy link

NoReturn is another name for Never. It is the "bottom type" in Python type system. You can think of it as equivalent to an empty union. NoReturn is compatible with all other types. No other type (other than NoReturn itself) is compatible with NoReturn.

@socketpair
Copy link
Author

Thanks for verbose explanation. I checked TypeScript:

const sys_exit: (code: number) => never = (code: number) => {
    throw new Error('message');
};

const x: number = sys_exit(42);

It also sees nothing wrong here.

image

I still don't understand why NoReturn / Never should be compatible with all types. For me, it must not by definition. Seems I don't understand something important in typing system. Will be great if you provide a link.

@KotlinIsland
Copy link
Contributor

KotlinIsland commented Sep 7, 2022

Because a value of Never can not exist, it is safe to use as a value of any type.

https://en.wikipedia.org/wiki/Bottom_type

def n() -> Never: raise Exception

print(n() + 1)

At runtime, this will not produce a TypeError, because the + will never be executed.

@socketpair
Copy link
Author

socketpair commented Sep 7, 2022

Why not make it incompatible with any type to detect obvious errors such as trying to use return value of noreturn functions ?

@Avasam
Copy link
Contributor

Avasam commented Dec 5, 2022

Why not make it incompatible with any type to detect obvious errors such as trying to use return value of noreturn functions ?

I think what you are asking for is more in the realm of a linting rule*: NoReturn is perfectly type-safe, but using the value of a function that does not return is a code smell and indicative of a potential bug / misunderstanding of the developer about what a function does.

I like the idea for that reason, idk if it belongs in mypy / pyright (maybe as an optional setting?). But few python linters are type-aware. I would personally turn on that rule if I had the option.

* It wouldn't the be the first time a type-checker has a rule that's not specific to typing (pyright has a few I like).

@KotlinIsland
Copy link
Contributor

KotlinIsland commented Dec 5, 2022

@Avasam Mypy complains on usages of functions that return None, so I think it would be make sense if this was included in mypy.

def foo() -> None:
  return

a: int | None = foo()  # error: "foo" does not return a value  [func-returns-value]

https://mypy-play.net/?mypy=latest&python=3.11&gist=e15787211c8a4115c82687bc15fee244

Perhaps the title of this issue could be updated to be something like "Warn on usage of Never/NoReturn result"

@erictraut
Copy link

Mypy is working correctly here. This isn't a bug. I recommend closing.

@AlexWaygood AlexWaygood closed this as not planned Won't fix, can't repro, duplicate, stale Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants