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

bad-exit-annotation (PYI036) triggers on typing.overload definitions #11044

Closed
tjkuson opened this issue Apr 19, 2024 · 5 comments · Fixed by #11057
Closed

bad-exit-annotation (PYI036) triggers on typing.overload definitions #11044

tjkuson opened this issue Apr 19, 2024 · 5 comments · Fixed by #11057
Assignees
Labels
accepted Ready for implementation bug Something isn't working

Comments

@tjkuson
Copy link
Contributor

tjkuson commented Apr 19, 2024

Keywords searched in the issue tracker: PYI036, overload

Running ruff check --select PYI036 --isolated using Ruff version 0.4.0 on the file

import typing
from types import TracebackType


class Foo:
    def __enter__(self) -> typing.Self:
        return self

    @typing.overload
    def __exit__(self, exc_type: None, exc_val: None, exc_tb: None) -> None: ...

    @typing.overload
    def __exit__(
        self,
        exc_type: type[BaseException],
        exc_val: BaseException,
        exc_tb: TracebackType,
    ) -> None: ...

    def __exit__(
        self,
        exc_type: type[BaseException] | None,
        exc_val: BaseException | None,
        exc_tb: TracebackType | None,
    ) -> None:
        pass

produces six PYI036 errors (three for each overload definition). The expected behaviour is that no PYI036 errors are raised, as these are overloads that describe legal calls rather than the complete implementation. This approach was recommended on this blog post.

@charliermarsh
Copy link
Member

@AlexWaygood - do you mind taking this one?

@charliermarsh charliermarsh added the needs-decision Awaiting a decision from a maintainer label Apr 20, 2024
@AlexWaygood
Copy link
Member

Yeah, good call — I think we should just skip any functions decorated with @overload for this check. Thanks for the report!

@AlexWaygood AlexWaygood added bug Something isn't working accepted Ready for implementation and removed needs-decision Awaiting a decision from a maintainer labels Apr 20, 2024
@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 20, 2024

On second thought, we can provide some validation for overloaded functions. Slightly more fiddly, but definitely doable.

@AlexWaygood AlexWaygood self-assigned this Apr 20, 2024
@charliermarsh
Copy link
Member

Thanks Alex!

@bswck
Copy link
Contributor

bswck commented Apr 22, 2024

Some "related off-topic" from me, since I recently went through a similar thing.

There should be no need to write overloads if we return None in either case.
That's the thing for a PEP to propose the ability of unpacking unions of same-sized tuples:

from __future__ import annotations

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from _typeshed import OptExcInfo  # tuple[None, None, None] | tuple[type[BaseException], BaseException, TracebackType]
    from typing_extensions import Unpack


class Foo:
    def __exit__(self, *args: Unpack[OptExcInfo]) -> None:  # should be legal, but isn't
        pass

What Ruff might or might not want to do is allow unpacking _typeshed.ExcInfo in the annotations of the __exit__ signature, not require either object or full signature. But that would only make sense if we always expect an error to occur 🤷‍♀️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants