Add rule for improper use of NoReturn

[SRv6d]

The NoReturn return type is commonly misunderstood and not very intuitive.

It should be used for functions that never return, either through unconditionally raising an exception,
or by exiting.

Yet, I commonly see it misused (and have misused it myself) to annotate functions that may raise or return using a union return value, eg:

def raise_conditionally(raise_exc: bool) -> None | NoReturn:
    if raise_exc:
        raise RuntimeError("raise_exc was true")
    return None

Although ideally ruff would ensure the full invariant(no use of NoReturn for functions that might do anything other than raise or exit), I haven't looked into how much work it would be to implement that and simply ensuring no use of union types with NoReturn would go a long way.

[Avasam]

This could fit https://github.com/PyCQA/flake8-pyi (which Ruff re-implements). I have myself introduced this incorrect union once by accident in typeshed (see python/typeshed#10819 )

[andersk]

All Ruff needs to check is that typing.NoReturn (and typing.Never) shouldn’t appear in a union, since that’s redundant: T | NoReturn is exactly equivalent to T.

Mypy already checks that NoReturn is not misused outside a union for a function that returns, and Ruff can’t do that without type information (since a function that always calls other -> NoReturn functions can be -> NoReturn).

[charliermarsh]

Will add.

[SRv6d]

@charliermarsh Much appreciated!

[SRv6d]

@charliermarsh Just FYI, I've just noticed that mypy as well as pylance honor usage of unions containing NoReturn in combination with overloads and will warn about code paths that follow a function call with a conditional NoReturn return value.

Given the following function:

from typing import Literal, NoReturn, overload


@overload
def raise_conditionally(raise_exc: Literal[False]) -> None:
    ...


@overload
def raise_conditionally(raise_exc: Literal[True]) -> NoReturn:
    ...


def raise_conditionally(raise_exc: bool) -> None | NoReturn:
    if raise_exc:
        raise RuntimeError("raise_exc was true")
    return None

This function is fine:

def is_reachable() -> None:
    raise_conditionally(False)
    print("This line will be executed")

But in this function the last line will be marked as unreachable:

def is_unreachable() -> None:
    raise_conditionally(True)
    print("This line will not be executed")

As per my understanding, this usage is incorrect, but still supported by both type checkers. Knowing this, do you want to keep the never-union rules as-is?

[andersk]

@SRv6d T | NoReturn isn’t incorrect, it’s just redundant, as it’s equivalent to T. You can replace None | NoReturn with None on line 14 (leaving as-is the NoReturn on line 10, which is not in a union), and you’ll get all the same results (mypy playground).

[SRv6d]

@andersk Thanks for clarifying, I falsely assumed that the return type of the function implementation had to be a union containing all the overloaded return types.