RET503 doesn't respect NoReturn

[hynek]

Given the following code:

import sys
from typing import NoReturn

def nr() -> NoReturn:
    sys.exit()


def f(x: int) -> int | None:
    if x == 5:
        return 5
    nr()

Results in this:

$ pipx run --no-cache ruff --version
ruff 0.0.275
$ pipx run ruff t.py --isolated --select=RET503
t.py:11:5: RET503 [*] Missing explicit `return` at the end of function able to return non-`None` value
Found 1 error.
[*] 1 potentially fixable with the --fix option.

Ruff will complain and add a return None behind the nr() line. This might look a bit silly, but it's kinda common e.g. in web apps when there's a helper function that just raises an exception. Given Mypy understands NoReturn, this leads to an error ping pong.

Cheers and thanks for the quick fixes of my previous bugs. :)

[NMertsch]

Shouldn't the return type of f be int | NoReturn? It is impossible for this function to return None. (Not sure if ruff would handle that in a better way, though)

[hynek]

My real example is a shared Union that defines what a web view can return. Basically Flask's ResponseValue with some extras.

[charliermarsh]

I know we should really support either, but in your case, is the annotation defined in your own code, or from a third-party module?

[hynek]

I'm not 100% sure what you're actually asking, because kinda both. :) It's defined in an external package to the application but that package is our own.

But also to NMertsch's point, I've made the example too complicated because I've misread the error message. This already triggers it:

import sys
from typing import NoReturn

def nr() -> NoReturn:
    sys.exit()


def f(x: int) -> int:
    if x == 5:
        return 5
    nr()

My real code looks roughly like this:

@bp.delete("/hosts/<name>/relationships/groups/<group>")
def delete_host_rel_group(name: str, group: str) -> RenderableResult:
    if svc.delete_group_host(get_uow(), group=group, host=name):
        return APIResult({}, status=204)

    resource_not_found(  # noqa: RET503
        "Host/Group relationship", f"{name}/{group}"
    )

With

Renderable = Union[dict[str, Any], list, int, str, bool]


@runtime_checkable
class RenderableResult(Protocol):
    status: int
    content_location: str | None

    def render(self) -> Renderable:
        ...

defined in a separate package.

[flaeppe]

Another example triggering this is exhaustiveness checking. Consider the following program

import enum
from typing import NoReturn


class MyEnum(str, enum.Enum):
    A = "A"
    B = "B"
    C = "C"


def exhausted(value: NoReturn) -> NoReturn:
    raise RuntimeError("Exhausted")


def do_something(value: MyEnum) -> str:
    if value is MyEnum.A:
        return "first"
    elif value is MyEnum.B:
        return "second"
    elif value is MyEnum.C:
        return "third"
    exhausted(value)

Which renders the following diff by RET503

def do_something(value: MyEnum) -> str:
    if value is MyEnum.A:
        return "first"
    elif value is MyEnum.B:
        return "second"
    elif value is MyEnum.C:
        return "third"
    exhausted(value)
+   return None

Which in turn yields a mypy error:

file.py:23: error: Statement is unreachable  [unreachable]
        return None
        ^~~~~~~~~~~

[charliermarsh]

We now respect NoReturn annotations in the next release, but it will only work for functions defined in the same file (so I'm not closing this).