mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Descriptors misbehave with `NoReturn`/`Never`, with `@overload`s, etc.

Open finite-state-machine opened this issue 1 year ago • 4 comments

Bug Report

To Reproduce

Gists: mypy-play.net pyright-play.net (pyright doesn't handle this well, issuing no warnings at all; it does respond to reveal_type() in the descriptors-used-as-intended case, suggesting it doesn't understand that directive is unreachable)

from __future__ import annotations
from typing_extensions import (
        NoReturn,
        Optional,
        overload,
        Self,
        Type,
        )

# Note: This is a simplified example.
#
#  - obviously, it would be more expedient to simply omit the definition
#    of '__get__()' for this trivial case
#
#  - similar problems affect implicitly calling '__set__()' through a
#    descriptor


class WriteOnlyDescr:
    '''some kind of write-only descriptor (to demonstrate the general issue)
    '''
    @overload  # descriptor accessible through class ('SomeClass')
    def __get__(self, owner: None, objtype: Type[object]) -> Self: ...
    @overload
    def __get__(  # access on an instance of 'SomeClass' (forbidden)
            self, owner: object, objtype: Optional[Type[object]] = None
            ) -> NoReturn: ...

    def __get__(
            self, owner: Optional[object],
            objtype: Optional[Type[object]] = None,
            ) -> Self:
        # allow access to this descriptor via the class:
        if owner is None:
            return self

        raise AttributeError

    # '__set__()' omitted for brevity


class SomeClass:
    '''a class with a write-only descriptor
    '''
    descr = WriteOnlyDescr()


descr = SomeClass.descr
inst = SomeClass()

if bool():

    # First case:
    #
    # First, we'll "spell out" the descriptor mechanics; this should
    # give the same results as using the descriptor in a natural way
    # (the second case)

                                # EXPECTED for the second case:
                                # ──────────────────────────────────────
    get = descr.__get__(inst)   # ← mypy: [var-annotated]
                                #     "Need type annotation for 'get'"
    reveal_type(get)            # ← (no mypy output; this is unreachable)
    _ = True                    # ← mypy: [unreachable]


else:                           # ↑ mypy output _should_ be identical ↑
                                # ↓    above and below this point     ↓
    # Second case:
    #
    # This _should_ give the  same output as above, since the first
    # lines of the two cases are virtually synonymous, while the
    # remaining lines are identical.

                                # ACTUAL for the second case:
                                # ──────────────────────────────────────
    get = inst.descr            # ← (no mypy output)
    reveal_type(get)            # ← mypy: "Revealed type is 'Never'"
    _ = True                    # ← (no mypy output)

Expected Behavior

Descriptors should behave the same way when used as intended and when the mechanics are spelled out (explicit calls to __get__(), etc.).

Actual Behavior

This isn't the case, especially when an argument has type Never or a function is marked NoReturn. (This can come up when @overloads are in use.)

Your Environment

  • Mypy version used: 1.8.0
  • Mypy command-line flags: --warn-unreachable is informative
  • Mypy configuration options from mypy.ini (and other config files): none
  • Python version used: 3.8, 3.12

Further context ...follows in a comment.

finite-state-machine avatar Feb 02 '24 23:02 finite-state-machine

Further context

This might help explain how this problem arises:

Gists: mypy-play.net pyright-play.net (seems less broken, but it's complaining about __init__() having an unknown return type when @overloaded)

from __future__ import annotations
from typing import *
from typing_extensions import (
        Never,
        Self,
        )

RO = Literal['RO']
RW = Literal['RW']

F_ro = TypeVar('F_ro', RO, RW)   # flag: is this 'MagicDescr' read-only?


class MagicDescr(Generic[F_ro]):
    '''some kind of write-only descriptor (to demonstrate the general issue)
    '''
    @overload
    def __init__(self: MagicDescr[RW], read_only: Literal[False]): ...
    @overload
    def __init__(self: MagicDescr[RO], read_only: Literal[True]): ...

    def __init__(self, read_only: bool):
        ...

    @overload  # descriptor accessible through class ('SomeClass')
    def __get__(self, owner: None, objtype: Type[object]) -> Self: ...
    @overload
    def __get__(  # access on an instance of 'SomeClass' (forbidden)
            self, owner: object, objtype: Optional[Type[object]] = None
            ) -> int: ...

    def __get__(
            self, owner: Optional[object],
            objtype: Optional[Type[object]] = None,
            ) -> Union[Self, int]:

        # allow access to this descriptor via the class:
        if owner is None:
            return self

        return 42

    @overload
    def __set__(
            self: MagicDescr[RO], owner: object, new_value: Never
            ) -> None: ...
    @overload
    def __set__(
            self: MagicDescr[RW], owner: object, new_value: int
            ) -> None: ...

    def __set__(self, owner: object, new_value: int) -> None:
        # do things
        return


class SomeClass:
    '''a class with a write-only descriptor
    '''
    ro = MagicDescr(read_only=True)
    rw = MagicDescr(read_only=False)



def ro_with_descriptor() -> None:

    inst = SomeClass()
    inst.ro = 42                    # no error (expected: [arg-type],
    _ = True                        #   just like in 'ro_with_e…_m…()')

def ro_with_explicit_mechanics() -> None:

    inst = SomeClass()

    SomeClass.ro.__set__(inst, 42)  # mypy: [arg-type] (as expected)
                                    #  ≈"Arg 2 to 'MagicDesc.__set__'
                                    #    has incompatible type 'int';
                                    #    expected 'NoReturn'"

    _ = True                        # (aside: shouldn't there be an
                                    #  '[unreachable]' here?)

def rw_with_descriptor() -> None:

    inst = SomeClass()
    inst.rw = 42                    # no error (as expected)
    _ = True

def rw_with_explicit_mechanics() -> None:

    inst = SomeClass()
    SomeClass.rw.__set__(inst, 42)  # no error (as expected)
    _ = True

finite-state-machine avatar Feb 02 '24 23:02 finite-state-machine

You seem to misunderstand what NoReturn means when used in a return type. It doesn't mean that an function (or overload) cannot be called. It simply means that when called, control will not return to the caller. Therefore, you cannot use this to indicate that access is "forbidden" for a particular call to __get__. If you want to indicate that it's forbidden, you should omit that overload signature.

erictraut avatar Feb 02 '24 23:02 erictraut

I do realize NoReturn is intended to represent sys.exit() and the like, but it should (in my understanding) signal to mypy that a callable with that return type will raise an exception, and that anything that follows is unreachable. (I'm unaware of any way to annotate a callable or @overload as "this shouldn't ever be called", so NoReturn seems the most appropriate annotation – it will, after all, raise at runtime.)

I must apologize for an oversight: in the second case, no error is issued about get needing a type annotation because the same variable is defined in the first case, and the error is issued there. So, mypy gets that right. (Renaming get to got in the second case illuminates this.)

On a bit of further experimenting, I think the bug (if any) is this: mypy doesn't realize everything following the implicit call to the descriptor's __get__() function (which has type Never/NoReturn) is unreachable.

Mypy will allow one to assign a variable to the descriptor read (type Never, as shown by reveal_type()), pass it to a function declared to take an argument of type Never, and manipulate the return value of that function. None of is reported as "unreachable", which strikes me as unintended.

We can replace everything from if bool(): on with the following [mypy-play.net] (results and commentary inline):

# (not shown: everything up to `inst = SomeClass()`)

# Second case (only):

got = inst.descr                    # ← mypy: [var-annotated] (same as first case)
reveal_type(got)                    # ← mypy: "Revealed type is 'Never'"
_ = True                            # ← (no mypy output, but this and all that
                                    #    follows should be unreachable)

def odd(arg: Never, /) -> int:
    return 42
    
odd_ret = odd(got)                  # ← (no mypy output; it seems like this
                                    #    shouldn't be allowed)
reveal_type(odd_ret)                # ← mypy: "Revealed type is 'int'"
                                    #   (surely, _this_ is unreachable?)

finite-state-machine avatar Feb 03 '24 05:02 finite-state-machine

(I very much appreciate the suggestion to omit the @overload entirely. The real-world use case is a @property-like descriptor which, by overloading __init__(), sets the TypeVar associated with the __set__() return type to NoReturn if no setter is provided. I can probably use an additional TypeVar to represent mutable/immutable, and, by annotating self in the __set__() overloads , effectively provide no signatures for that method in the immutable case. This very likely solves my problem, although I suspect there's still something funny going on with unreachability, so I'll leave the issue open for now – subject to your judgment, of course.)

finite-state-machine avatar Feb 03 '24 05:02 finite-state-machine